cfpb / django-flags

Feature flags for Django projects
https://cfpb.github.io/django-flags/
Creative Commons Zero v1.0 Universal
260 stars 32 forks source link

Database-dependent check fails with no database #18

Closed chosak closed 5 years ago

chosak commented 5 years ago

The Django check introduced in #16 has a dependency on the database due to the way that it validates flag conditions. Unfortunately Django checks may run at times when the database hasn't been created, or is unavailable.

For example, starting in a fresh project with no database and running manage.py migrate will try to execute the Django checks before the database tables are created. This will result in a stack trace like this:

  File "/home/venv/lib/python2.7/site-packages/flags/checks.py", line 13, in flag_conditions_check
    flags = get_flags()
  File "/home/venv/lib/python2.7/site-packages/flags/sources.py", line 103, in get_flags
    source_flags = source_obj.get_flags()
  File "/home/venv/lib/python2.7/site-packages/flags/sources.py", line 77, in get_flags
    for o in self.get_queryset():
  File "/home/venv/lib/python2.7/site-packages/django/db/models/query.py", line 250, in __iter__
    self._fetch_all()
  File "/home/venv/lib/python2.7/site-packages/django/db/models/query.py", line 1118, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/home/venv/lib/python2.7/site-packages/django/db/models/query.py", line 53, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch)
  File "/home/venv/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 899, in execute_sql
    raise original_exception
django.db.utils.OperationalError: no such table: flags_flagstate

It looks like #17 is proposing a fix for this, although there might be a better/alternate way to handle that's a little more targeted (i.e. ignoring all exceptions would hide any real database errors).

benefacto commented 3 years ago

We're experiencing this issue on a project we're working on and it's making the package unusable for us. It occurs when we load the templates that use flags and on the admin dashboard when we navigate to the flag states. We don't experience the error locally--just on our hosted application (on Heroku):

UndefinedTable: relation "flags_flagstate" does not exist
LINE 1: ...state"."value", "flags_flagstate"."required" FROM "flags_fla...
                                                             ^

  File "django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)

ProgrammingError: relation "flags_flagstate" does not exist
LINE 1: ...state"."value", "flags_flagstate"."required" FROM "flags_fla...
                                                             ^

  File "django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "django/contrib/admin/options.py", line 606, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "django/contrib/admin/sites.py", line 223, in inner
    return view(request, *args, **kwargs)
  File "django/contrib/admin/options.py", line 1645, in add_view
    return self.changeform_view(request, None, form_url, extra_context)
  File "django/utils/decorators.py", line 45, in _wrapper
    return bound_method(*args, **kwargs)
  File "django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "django/contrib/admin/options.py", line 1529, in changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "django/contrib/admin/options.py", line 1586, in _changeform_view
    form = ModelForm(initial=initial)
  File "flags/forms.py", line 25, in __init__
    (f, f) for f in sorted(get_flags().keys())
  File "flags/sources.py", line 183, in get_flags
    source_flags = source_obj.get_flags()
  File "flags/sources.py", line 136, in get_flags
    for o in self.get_queryset():
  File "django/db/models/query.py", line 274, in __iter__
    self._fetch_all()
  File "django/db/models/query.py", line 1242, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "django/db/models/query.py", line 55, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "django/db/models/sql/compiler.py", line 1142, in execute_sql
    cursor.execute(sql, params)
  File "django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "django/db/backends/utils.py", line 76, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
  File "django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)

Essentially, everything works fine locally and we're using the example in two of our templates similar to the following: base.html

{% load feature_flags %}
{% flag_enabled 'MY_FLAG1' as my_flag1_enabled %}

{% if my_flag1_enabled %}
  <div class="flagged-banner">
    I’m the result of a feature flag.   
  </div>
{% endif %}

template.html

{% extends 'base.html' %}
{% load feature_flags %}
{% flag_enabled 'MY_FLAG2' as my_flag2_enabled %}

{% if my_flag1_enabled %}
  <div class="flagged-banner">
    I’m the result of a feature flag.   
  </div>
{% endif %}

Then, in our settings, we're configuring the flags' conditions:

## Feature flags configuration
FLAGS = {
    "MY_FLAG1": [
        {"condition": "parameter", "value": "enable_flag1"},
    ],
    "MY_FLAG2": [
        {"condition": "parameter", "value": "enable_flag2"},
    ],
}

This is meaning that we're going to have to use environment variables to roll our own feature flags rather than using this library. We'd prefer to use this library as it has plenty of functionality and seems well maintained. We'd appreciate any help anyone can offer.

willbarton commented 3 years ago

@benefacto A couple of things, first are you setting FLAG_SOURCES to just ('flags.sources.SettingsFlagsSource', )?

Second, if you're using the Django admin, it won't show you non-database flag states (and indeed, will likely error as you've described). If you use the Wagtail CMS, our wagtail-flags library is a bit more robust in that regard (the Django admin is somewhat fragile to customize for non-database information).

We do have a Django Debug Toolbar panel that should show flag conditions from any source, database or settings.

I hope all of this helps!

EDIT: A third question, on your Heroku deploy, are you running migrate? It sounds like the tables aren't getting created (which, unfortunately, they will regardless of whether you've configured Django Flags to use database conditions or not)?

All that being said, if you're only going to define flags in settings, the Django admin won't be useful, and I'd actually recommend unregistering the FlagState model with the admin (i.e. admin.site.unregister(FlagState)).

benefacto commented 3 years ago

@willbarton Thanks for the response! Upon further investigation, we discovered that it was happening with other migrations and was actually an issue with our Heroku deploy process. We were deploying different branches that have different migration trees to the same database which was the likely cause of this issue. Running migrate --run-syncdb worked around the issue for now but we will have to probably make additional changes to our deployment process. I don't believe this was actually an issue with django-flags but I appreciate the prompt response.