Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Support variant-less rollouts #345

Closed
dcramer opened this issue Apr 7, 2020 · 10 comments · Fixed by #354
Closed

[feat] Support variant-less rollouts #345

dcramer opened this issue Apr 7, 2020 · 10 comments · Fixed by #354

Comments

@dcramer
Copy link
Contributor

dcramer commented Apr 7, 2020

In contrasting open source feature systems flagr is certainly one of the more mature options. The one caveat I found was its quite complex for a generic boolean feature flag. The scenarios that I think could be simpler are:

  • global on/off: could respect the default on/off state.
  • % of entity_id on/off: could respect the segment rollout with no distribution plan

You could achieve the latter if the API returned some kind of information saying "segment is on with rollout plan and no variant".

@fenriskiba
Copy link
Contributor

If parsing a string is a concern, couldn't you just add a bool to the variant attachment? Just seems like a simpler way of handling it rather than adding the complexity of a whole new type of rollout.

image

@dcramer
Copy link
Contributor Author

dcramer commented Apr 7, 2020

@fenriskiba that defeats what im saying in this ticket - having to add any variant is already extra complexity for a basic switch. I only need to know if it matches the rollout of a segment. Otherwise yes I can simply match on 'variantId' in the response to say "is it on at all", but it now requires training folks to add "random variants of whatever you want".

@fenriskiba
Copy link
Contributor

Okay, I see where I had a disconnect. I was thinking through using the evaluations as a problem and you were looking at training users of the UI. I'm also referring to the complexity of the internal Flagr app, not the user experience (not sure if that got across since the response refereed to complexity of the user experience).

So then the follow up question I'd have would be to ask if default variants/segments/distributions would help solve the same problem? I don't necessarily think what your proposing would be bad, but I'm just wondering if a smaller change could provide the same value and be easier for someone to contribute.

@zhouzhuojie
Copy link
Collaborator

I'm in favor of a consistent flag-segement-variant model, that reduce the complexity of maintaining a new "simple boolean flag" type.

And to address the concerns, there are two parts of the complexity for "simple boolean flag":

  • Hard to create: you have to create a flag, create a variant, create a segment, and then create a distribution for that single variant.
  • Hard to evaluate: you have to check either "variandID" or "variantKey" from the evaluation response.

What do you think we expose an endpoint that create a "simple boolean flag", and in the backend we assemble those actions for the convenience but still result in a normal flag-segment-variant model?

E.g.

  • POST /api/v1/flags?creation_type=simple_boolean_flag (or POST /api/v1/simple_boolean_flags)
  • Backend creates a flag with 1 variant, 1 segment, 1 distribution (100% to that variant), and you only need to control two variables:
    • Enabled: on/off
    • Segment Rollout Percent: 0 - 100 %
  • For evaluation, you only need to check if the variantID or variantKey is null or not based on the two variables defined above.

UI can stay the same for now, since people may want to extend a simple boolean flag to more variants and more segments.

@dcramer
Copy link
Contributor Author

dcramer commented Apr 7, 2020

@zhouzhuojie that could definitely work - whats your thoughts on determining on/off? I do still think it'd be nice if the API exposed "user in rollout but no variants", and you can get the latter (no variant id) but you cant get the former, as the segment is returned even if the rollout doesnt trigger.

@dcramer
Copy link
Contributor Author

dcramer commented Apr 9, 2020

what about something like this?

image

@dcramer
Copy link
Contributor Author

dcramer commented Apr 9, 2020

Also if you're supportive, my plan is to accept some kind of 'Simple' boolean via the existing flag endpoint (per the suggestion), and then create the above objects as described. Will throw up a PR later today likely.

@zhouzhuojie
Copy link
Collaborator

LGTM, and thanks for the new creation button!

Thinking of the API design, maybe we can add a new field creationType in the payload body of https://checkr.github.io/flagr/api_docs/#operation/createFlag, with backward compatibility of an empty value.

Compared with assembling existing flags/segments/variants endpoints from UI:

  • API client can start using this endpoint
  • Better testing at the backend side instead of assembling 4-5 API requests from frontend UI.
  • The new creation button can be as simple as calling this endpoint with the field creationType = simple boolean button

@dcramer
Copy link
Contributor Author

dcramer commented Apr 9, 2020

@zhouzhuojie in building this I had an interesting idea: exposing the creationType more as a template param. That would give the opportunity to extend it in the future with a more obvious goal. Does that seem reasonable?

fwiw I just added this on the create flag endpoint as an optional param so:

POST /flags {
  key: afasdf
  template: "simple"
}

@zhouzhuojie
Copy link
Collaborator

@zhouzhuojie in building this I had an interesting idea: exposing the creationType more as a template param. That would give the opportunity to extend it in the future with a more obvious goal. Does that seem reasonable?

fwiw I just added this on the create flag endpoint as an optional param so:

POST /flags {
  key: afasdf
  template: "simple"
}

sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants