-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Proposal for feature flags #636
Comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
@micahflee's PR for Stripe integration (#595) is rather large at +3.5k LOC, and it's taking quite a long time to merge (rightly so as it's complex). For such large features, it might be worth merging them in piecemeal and disabling functionality via feature flags. This would allow us to quickly iterate and keep features only to dev. We could also avoid too many migrations because the features could be completely turned off in prod so the DB would never need such a schema. We could even feature flag columns in
Model
s.Describe the solution you'd like
We could have a simple module called
feature_flags.py
that looks like this:Alternatively, we could hard code functions like
is_feature_foo_enabled()
to avoid typos and have a single place they're pulled from.We could use it in templates and even models like
Describe alternatives you've considered
There are some ways to enable feature flags on a per-user or percent-of-users basis, but I don't care about those right now, only all or nothing features.
Conflicts
This would conflict with migration tests that ensure the
db.create_all()
schema matches the head of the Alembic migrations. We don't have that yet, and we could alter the test fixtures for those tests to disable features so the schemas match.Tests that use the
--alembic
flag we added topytest
would fail because obviously the migrations don't exist yet. We could add the same feature flag logic into the migrations (and then remove it once the feature is finalized), but that's starting to get really fucky and error prone.The text was updated successfully, but these errors were encountered: