Added autocreate settings to fix #44

[wolph]

This patch makes it possible for Waffles to automatically create switches upon usage so you don't have to manually create them.

This fixes ticket #44

[travisbot]

This pull request passes (merged 7c77026 into ee9bb1d).

[katzj]

I was about to do something to handle keeping track and creating new flags as a manage.py command to be done on deploy, but this looks even nicer. Any chance of getting it merged?

[jsocol]

Hey @wolph, I think #50 is a better way to address the underlying issues here. That's more the direction I think the project should evolve.

[wolph]

I think a combination would be needed. This fix might not be the best one, but it does give the (imho large advantage) that you can see all defined switches.

A good combination could be to optionally scaffold/list all switches but I don't see any way how that could work without code analysis.
Defining them in your settings file requires you to do double work so that is not ideal either. Also, that way you cannot generate them dynamically (for example based on content type).

[ekohl]

How about ekohl@a5ab978 for autocreate defaults?

[ekohl]

Tests are in ekohl@716476a.

[ekohl]

@jsocol could you have a look at the patches I linked or would you prefer I submit a separate PR? Or should I make a PR to @wolph to have him update this PR?

[ekohl]

@jsocol ping

[jsocol]

@ekohl It's probably easiest to open a new pull req, unless @wolph has time to merge your patches here.

Does anyone have a solution to the thundering herd/race condition issue here? On deploying a new flag to a moderately busy site, autocreate is likely to cause a brief spike in errors as multiple processes try to create it.

[wolph]

At the moment I am quite busy, in a week or 2 I should have some time again.

As for the race condition, is that really a problem? Generally these kind of flags would be generated upon a new deploy. So unless you want the entire website to go down during deployment you would do that per app server anyhow and in that case it shouldn't be a problem anymore.

[jsocol]

As for the race condition, is that really a problem?

It's something that at the very least would need to be documented as a warning along with the setting. It will probably only affect high traffic sites but it should still come with a "know what you're doing" label.

Generally these kind of flags would be generated upon a new deploy. So unless you want the entire website to go down during deployment you would do that per app server anyhow and in that case it shouldn't be a problem anymore.

The only way there's no race condition is if there's only one, single-threaded server process. (Or one process per app server, assuming a rolling deploy.)

[wolph]

It's something that at the very least would need to be documented as a warning along with the setting. It will probably only affect high traffic sites but it should still come with a "know what you're doing" label.

Fair enough, a warning label should be in place :)

The only way there's no race condition is if there's only one, single-threaded server process. (Or one process per app server, assuming a rolling deploy.)

I have to admit that I didn't look at this code since the pull request but it is indeed kind of crappy in terms of thread safety. Even though the race condition might not always cause problems, the current code has zero protection from race conditions. It should use get_or_create instead which does handle race conditions cleanly.

The current code is not safe for production usage. Even with very small sites it can cause problems.

[jsocol]

+1 to get_or_create, I always forget about that but it would simplify things in general. This also makes me note that there's no negative caching, and their probably should be, otherwise missing flags hammer the DB. That also means digging into invalidation code... Oh great, I've made more work for myself :)

[wolph]

I'm willing to create a new/better patch and/or merge the patch from @ekohl in a few weeks but I simply don't have the time for it right now. And in retrospect this code really isn't production ready which is why I closed the request for the time being ;)

As for the negative caching... annoying problem regardless, especially since you don't want to cache these things forever. I guess the patch should address both...

[jsocol]

Don't worry about the cache issue, even starting to look at this I am just creating a huge list of stuff. It might be worth holding off on this for a few weeks anyway, and letting me find some time to do some bigger clean-up work.

[ekohl]

I'm willing to look into this because we're looking into using waffle and I sort of see this as a requirement since we have a few developers and I'm sure that it's too much effort to manually create all flags. I'm working on a new PR based on this + my patches.

[jsocol]

@ekohl How many flags do you think you'll be creating at once? My experience has been that these tend to get added one at a time as a/b tests come up.

[wolph]

Can't answer for @ekohl, but in my case I prefer that developers/designers/etc... just add all new features and things with a flag so they can be disabled if for some reason they cause problems.

Disabling a flag is a lot faster than deploying a new version to 20 application servers, 10 celery workers and similar. The amount of new flags added is therefore difficult to predict.

[jsocol]

Sure, that makes sense, but you're still not bulk-creating flags.

[ekohl]

@jsocol all developers have their own environment, there's automated testing, staging and production. I expect we want to default to the same value.

[jsocol]

I'm starting to write something up about some of the changes I want to make to clean things up more generally. I'd suggest at least waiting til I finish laying that out, before investing a ton more time here. Just to avoid conceptual merge conflicts.

[ekohl]

You're too late because I just opened #95 which squashes my patches and uses get_or_create.