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

Add initial attempt at env-var based flag specification (and tests). #187

Closed
wants to merge 1 commit into from

Conversation

ntoll
Copy link
Member

@ntoll ntoll commented Dec 4, 2015

This PR introduces a new capability. It is unfinished (only flagging is supported) and this PR is where discussion of ongoing development can be had with upstream project owner. This work is being done with @rossbruniges.

What?

I'm working for a client whose Django website is "stateless" (i.e. does not have a database backend). This branch makes it easy to use three arbitrary "buckets" for specifying flags defined in environment variables: ALPHA, BETA and ALL, thus making it possible to use Waffle with websites that are not backed by a database.

Why?

My client wants to use feature flags and Waffle is awesome. Furthermore, not all websites are "typical" in that they use a database for storing state.

How?

There are three arbitrary buckets: ALPHA, BETA and ALL. ALPHA and BETA have named users associated with them and flags set for each bucket will only be active for those users. ALL related flags apply to all users. The choice of arbitrary bucket names was made to reflect the common pattern of rolling out a feature first in "alpha", then "beta" and then to "all" users. It is assumed that once a feature has been around long enough and there's no requirement for it to be on/off at short notice, then the flag related code should be removed from this feature.

We use the following environment variables for specifying flag/user state:

  • WAFFLE_USE_ENV_VARS - a simple truthy flag to indicate that waffle should use the env vars instead of the database when checking if a flag_is_active.
  • WAFFLE_ALPHA_USERS - a comma separated list of usernames for those in the ALPHA bucket (the stateless Django app will need to implement a faux-request.user.username to identify the individual users).
  • WAFFLE_BETA_USERS - as with WAFFLE_ALPHA_USERS but for BETA users.
  • WAFFLE_ALPHA_FLAGS - a comma separated list of flags in the ALPHA bucket.
  • WAFFLE_BETA_FLAGS - a comma separated list of flags in the BETA bucket.
  • WAFFLE_ALL_FLAGS - a comma separated list of flags in the ALL bucket.
  • WAFFLE_SWITCHES - TBD.
  • WAFFLE_SAMPLES - TBD.

This is most definitely a first draft of this functionality. I'm not precious about my code: comments, constructive critique and ideas are most welcome. "Not interested" is also a valid response! :-)

In any case, my client needs this feature so I'll be working on it over the coming weeks and, if useful, would like to make the work available upstream.

@peterbe
Copy link

peterbe commented Dec 4, 2015

Why not leave the env var parsing to decouple and then use the django settings framework instead?

(hi! by the way @ntoll We meet again in django-land!)

@jsocol
Copy link
Collaborator

jsocol commented Dec 4, 2015

@rossbruniges mentioned that y'all might be doing something like this, and I warned that I wasn't likely to merge it. With this in hand, I see a couple of significant issues that make it even less likely.

  • Introducing "buckets" like this is a huge organizational shift. Waffle-with-env-vars would be a dramatically different project than Waffle-with-a-db.
  • So is introducing a requirement to restart running processes to change flag state.
  • So is identifying users by name instead of ID.
  • There is no support for percentage rollouts, groups, or other user states, or a route toward supporting UA Switching #3 or Add IP/subnet to flags #37. Adding the parts that make sense would require a big investment in parallel code. which is a lot of maintenance to take on, and there would never be parity.

I think the best option is probably to start a new project or to fork Waffle. Waffle aims to support 90%/mainstream usecases, and "django without a database" doesn't fall into that. Plus, if someone did have a database but wanted to use env vars to configure feature flags, they'd still be able to via a new project.

A new project would be able to delete hundreds of lines of code here and expose the same sort of API in templates. The body of switch_is_active could be replaced with return bool(int(os.getenv(name, 0))). sample_is_active would get similarly shorter.

Or, like @peterbe said, rely more heavily on the settings framework.

@ntoll
Copy link
Member Author

ntoll commented Dec 4, 2015

@jsocol quite understand - as you probably realise, much of the way this works depends on my client's "context". I think the best way forward is to fork into my client's organisation and they can run with it. Thanks for the suggestions too.

@peterbe hey hey matey. Yes, settings framework could work. I'll look into that. ;-)

@jsocol
Copy link
Collaborator

jsocol commented Dec 4, 2015

much of the way this works depends on my client's "context".

I'm also not keen to take on the maintenance cost of all this new code for a single client that—I'm guessing—isn't going to contribute resources to help keep Waffle running after this project. I'm going to close this out.

If you do fork it to a public place, all I ask is that it not be called "waffle". (I have no mechanism to enforce that, just saying "please.")

@jsocol jsocol closed this Dec 4, 2015
@ntoll
Copy link
Member Author

ntoll commented Dec 7, 2015

@jsocol ok I've forked it and renamed it "Waff" (as in Waffle, but without a back-end). For reference, it's here: https://github.com/ntoll/waff but will probably become re-owned by DigitalInnovation (part of Marks and Spencer - my client) when I've finished the work.

@jsocol
Copy link
Collaborator

jsocol commented Dec 7, 2015

Cool. I'd really encourage you to drop all the database stuff and strip out those parts of the code, it'll be a lot easier to maintain.

@ntoll
Copy link
Member Author

ntoll commented Dec 7, 2015

Totally agree. That'll take some time, but the current "cut" of the code is usable.

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 this pull request may close these issues.

None yet

3 participants