jazzband / django-waffle

A feature flipper for Django
https://waffle.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.13k stars 257 forks source link

Be able to define switches and default states in settings #50

Open tobych opened 12 years ago

tobych commented 12 years ago

Just like Gargoyle:

http://gargoyle.readthedocs.org/en/latest/install/index.html#default-switch-states

Although I think Gargoyle only adds the switches when they're used. So it's autocreation (see the other Issue), but with the autocreation using stuff from settings if available.

GARGOYLE_SWITCH_DEFAULTS = { 'new_switch': { 'is_active': True, 'label': 'New Switch', 'description': 'When you want the newness', }, 'funky_switch': { 'is_active': False, 'label': 'Funky Switch', 'description': 'Controls the funkiness.', }, }

When I used Gargoyle I always committed an addition to this setting along with whatever switch-controlled feature I was adding, to provide documentation and so noone would need to add the switch manually. I'd actually prefer all the switches to be added at startup though (nonexistant ones).

Currently I'm getting away with ./manage.py loaddata waffle.json each time I deploy, but that's not ideal.

I'll probably end up doing this myself as we'll want this in production soon. Comments welcome though about what I might best do and how I might do it. Perhaps this should be done with WoLpH (Rick van Hattem) and #44.

I'm ONLY using switches, currently, by the way.

jsocol commented 12 years ago

I'm ONLY using switches, currently, by the way.

Anything that works for switches, and is applicable, will need to apply to all three types.

tobych commented 12 years ago

So James, shall I go ahead and work on this? I'd start with just a write-up of what I'd do, perhaps with documentation, and a little about implementation. I'd check-in with Rick (see above), too.

Sveder commented 9 years ago

(2 years later) Are you guys still in favor of this issue? I'm thinking about scalable ways to deploy waffle switches and sync our dev, staging and production environments with the same switch names. Auto creation seems too implicit to me. I can try and polish the hack I'm currently writing into a pull request if there is someone here to review and patch it.

jsocol commented 9 years ago

I've never been 100% on board with doing this. I can't say for sure why I never answered @tobych two years ago (sorry, Toby!) but that's probably part of it.

One thing to keep in mind is I'm really unhappy with the core of waffle right now. It's one of my first Python projects and that shows, in mundane ways like code organization and bigger, harder to approach problems like extensibility. Every flag can't have two hundred attributes but that's the road we're on right now.

Another issue is that waffle is very much about separating features from deployments. So this is a weird situation, because either settings would be use-once, to create rows in the DB, and then they'd be cruft—and for a lot of people, cruft you couldn't remove if you wanted to be able to use deployment infrastructure consistently—or they'd be a different source of truth, one that required deploys to update.

I don't like either of those options, but I dislike the latter less. People are clearly interested in being able to define flags/switches/samples in settings. I think it follows a principle of least-surprise that if you add a flag to the settings, then change some aspect of it in the settings, that aspect should change deterministically. It doesn't create a situation where you would get an annoying "add a setting, deploy, remove a setting" workflow. The former (use-once settings to create the DB object) has all these problems.

Settings-as-alternate-source has problems, too. Beyond code complexity, it adds confusion ("I changed the setting, why didn't it change? oh it's in the DB") and doesn't actually put the objects in the DB to be used and changed, missing the autocreate problem completely. And it further ties users into a model that isn't aging well by copying all the problems over from database schemas to configuration dicts.

I'm going to bring in #44 here. @WoLpH described creating objects manually as "kind of tedious and unneeded". But AIUI, he's talking more about a single setting that means something like "when *_is_active is called and the DB object doesn't exist, instead of caching 'no-object', create the object with the default values". This kind of simple autocreate addresses "tediousness" and certainly spelling errors, but it doesn't help in a case where you need the object to have a different value immediately—that would require creating it, manually, before the code was live. "Simple" autocreate as in #44 has the fewest issues, but perhaps the lowest value.

(It's worth pointing out that there are a series of management commands available that can be used to manipulate flags either by hand or from a script.)

I put this in milestone 0.11 at some point with the intention of talking about it. Because while I've never really heard or thought of a proposal that I think fits well, I also haven't been convinced it's a bad idea.

Lastly, I want to make sure we're on the same page about what problem or problems we want to solve. I think there are three or four mixed up here. Maybe I'm missing or confused about some of them. Here, as user stories:

So, what do y'all think?

wolph commented 9 years ago

Disclaimer: it's been a while since I used Waffle so I'm not a 100% on the features.

A system like this should (imho) have, at least, the following features:

As for user story number 3, it's a bit of a contradictory story compared to some others. If you want to store the settings on the filesystem it would be impossible to change live. If you want to store them in the database you would need something like reversion (or your own implementation) for versioning.

Sveder commented 9 years ago

Ok, so I'll give you my 2 cents, but I've been working with waffle for a few hours, so take this with a grain of salt.

The issue I think most missing from auto-creation or the command line is the note/description field. I'm going to let "kinda-technical" people (read - not programmers) manipulate flags in the admin and so I want a note there for them to understand the repercussions. When I looked at the various issues open, this one caught my eye as a good solution that didn't involve me doing a datamigration.

I definitely understand why you don't want to add another bunch of settings ("waffle_use_db", "waffle_use_settings", etc). I also can't really speak to how hard/easy it will be in regard to how the code reads. For me, it currently boils down to creating a south data migration and adding the switches, adding note/description option to the commands or writing a version of this feature. The first one is a ton less work, but the second and third ones will be better in the future and might help other people.

Ninja edit: I'm generally for adding things to settings as they are a solved problem. People know how to use them and there is a lot of documentation about how you can version them, create different settings for different environments and maintain them.

jsocol commented 9 years ago

Thanks @WoLpH! I didn't realize you were so far out, thank you for the insight and if you mute this thread, I totally understand!

W/r/t these three properties, I think we're OK.

Regardless of whether the flag exists, it should give a proper result (i.e. false, predefined, whatever) without errors and with caching.

Flags/switches/samples that are not defined are always defined by the *_DEFAULT settings (which are all False by default). It's well-defined behavior.

They don't cache misses correctly right now and I think that's because of the signal strategy, which I've just decided to replace with something much simpler. (See #137)

Setting defaults before using, either through a settings file or the database.

Doable via the database.

Live changes. When enabling flags and noticing they don't work and/or cause errors, they should be removable instantly.

Doable via the database.

@Sveder, you actually bring up a very good point here: data migrations. With south, or, especially, with Django 1.7, they are a good technical solution. They can be run automatically, fixing the manual typing problems, and set whatever attributes make sense, whether it's meaningful state or simply the notes. They can be created and run ahead of time, stored in SCM, run on all environments.

Maybe the answer is to add some support—management command, maybe a script—that makes it easier to generate data migrations.

It's not immediately clear where you'd store them. I suppose each project would have to pick a first party app where they'd live. But that could be a setting or command-line option.

I'm not sure what programmatically creating data migrations is like right now, but it's definitely something I could look into. At worst, it'd mean writing out the correct Python file, which is not the most robust idea, but migrations should generally be designed to be forward compatible so I think the fragility would be OK.

mwarkentin commented 9 years ago

Just came across this from some comments in the other thread.. we went through some of the same questions as well (although after we used fixtures, then data migrations for a while, I started to push for just using the database, and writing your code in a way that would work if the flag / switch didn't exist). Someone ended up writing a small app, although it was never open-sourced.

Here are some examples of the fixture (don't do it) / data migrations (this was annoying to generate manually - if it was automated it might be better) we used if that's helpful to anyone: https://gist.github.com/mwarkentin/5f8dcea9a507b7675316

Here's the basic gist (haha) of the app: https://gist.github.com/mwarkentin/927b9b064b3d0ecf510a

I do find that building your flags this way makes them harder to clean up .. eg. sometimes people reference flags by their waffle-iron constant, rather than their name, which makes searching for them harder.

If something like this would be useful, I could see about open-sourcing the app.

cleor41 commented 6 months ago

+1 to adding something like this.

I flipped through different solutions e.g. automatically creating migrations named via the hash of a custom waffle dict and piggy backing off makemigrations. I ultimately ended up just creating a script to create the flags via the Waffle CLI and calling it during the startup.sh.

One of the reasons you would need something like this is because we use some switches as frontend only, they'll never be created in this case because the backend will never hit a line of code that uses the the switch.

clintonb commented 6 months ago

@cleor41 why not use data migrations?

cleor41 commented 6 months ago

Migrations are a totally valid approach!

More background, I don't want every developer on my team to have to mess around with migrations directly. So at one point I adopted the approach of using a custom config to define the defaults etc. Migrations are only applied once and so a new one needs to be created when the config changes. That's why the hash of the config is used as the migration file name. This would happen automatically as the code runs when makemigrations runs. Now with this approach, although it works easy enough for switches, if we decided to add flags and samples, then we would need to add more capabilities to the config to make it robust and support all of the functionality. It seemed like more complexity than what was necessary.

Instead of going through the trouble, I decided to just run a script when the server starts. Now developers just define the appropriate Waffle CLI command in the script. The script runs through all of the commands in a loop and calls them. All of the capabilities are provided via the Waffle CLI command and developers just need to know Waffle - which they would have to know in the migration case anyway.

Although none of this is needed if Waffle supported a non migration non global default state setup out of the box. I just needed to fill the gap for the time being and this seemed like the fastest solution. I actually kind of prefer this now as it's more direct with more freedom than my config approach. For the switch case you just need the following code, which is a pretty easy paradigm to follow.

from django.core.management import call_command
commands = [
  ["waffle_switch", "<SWITCH_NAME>", "on", "--create"]
]
for command in commands:
  call_command(*command)

EDIT: To use this approach you have to make sure the switch doesn't already exist - otherwise it'll change the value to be what it is in during instantiation when the server restarts. You can do this by parsing "manage.py waffle_switch - l"