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

Setting user based flag conditions prevents migrations running on a fresh database #96

Open
moaxey opened this issue Mar 8, 2022 · 4 comments

Comments

@moaxey
Copy link

moaxey commented Mar 8, 2022

My CI always starts with a fresh database. This user condition gave me the desired behaviour, and worked when adding the Django flags app and migrating from an existing database:

 FLAGS = {
     'FLAG_NAME': [
         {'condition': 'user', 'value': 'test_user'},
         {'condition': 'boolean', 'value': True, 'required': True}
     ]
 }

But the user condition would be tested before the user table was created when running migrations on a new database.

Traceback (most recent call last):
 File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 85, in _execute
   return self.cursor.execute(sql, params)
psycopg2.errors.UndefinedTable: relation "users_user" does not exist
LINE 1: ...ers_user"."date_joined", "users_user"."name" FROM "users_use...
                                                            ^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
 File "/app/manage.py", line 31, in <module>
   execute_from_command_line(sys.argv)
 File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 425, in execute_from_command_line
   utility.execute()
 File "/usr/local/lib/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute
   self.fetch_command(subcommand).run_from_argv(self.argv)
 File "/usr/local/lib/python3.9/site-packages/django/core/management/base.py", line 373, in run_from_argv
   self.execute(*args, **cmd_options)
 File "/usr/local/lib/python3.9/site-packages/django/core/management/base.py", line 417, in execute
   output = self.handle(*args, **options)
 File "/usr/local/lib/python3.9/site-packages/django/core/management/base.py", line 90, in wrapped
   res = handle_func(*args, **kwargs)
 File "/usr/local/lib/python3.9/site-packages/django/core/management/commands/migrate.py", line 75, in handle
   self.check(databases=[database])
 File "/usr/local/lib/python3.9/site-packages/django/core/management/base.py", line 438, in check
   all_issues = checks.run_checks(
 File "/usr/local/lib/python3.9/site-packages/django/core/checks/registry.py", line 77, in run_checks
   new_errors = check(app_configs=app_configs, databases=databases)
 File "/usr/local/lib/python3.9/site-packages/flags/checks.py", line 30, in flag_conditions_check
   condition.fn.validate(condition.value)
 File "/usr/local/lib/python3.9/site-packages/flags/conditions/validators.py", line 44, in validate_user
   UserModel.objects.get(**{UserModel.USERNAME_FIELD: value})
 File "/usr/local/lib/python3.9/site-packages/django/db/models/manager.py", line 85, in manager_method
   return getattr(self.get_queryset(), name)(*args, **kwargs)
 File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 435, in get
   num = len(clone)
 File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 262, in __len__
   self._fetch_all()
 File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 1354, in _fetch_all
   self._result_cache = list(self._iterable_class(self))
 File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 51, in __iter__
   results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
 File "/usr/local/lib/python3.9/site-packages/django/db/models/sql/compiler.py", line 1202, in execute_sql
   cursor.execute(sql, params)
 File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 67, in execute
   return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
 File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 76, in _execute_with_wrappers
   return executor(sql, params, many, context)
 File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 85, in _execute
   return self.cursor.execute(sql, params)
 File "/usr/local/lib/python3.9/site-packages/django/db/utils.py", line 90, in __exit__
   raise dj_exc_value.with_traceback(traceback) from exc_value
 File "/usr/local/lib/python3.9/site-packages/django/db/backends/utils.py", line 85, in _execute
   return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: relation "users_user" does not exist
LINE 1: ...ers_user"."date_joined", "users_user"."name" FROM "users_use...
                                                            ^

Do I need to add these flags in a migration themselves instead of in code?

@willbarton
Copy link
Member

@moaxey Hmm, I don't think you should need to do anything in migrations. This looks like a bug in Django-Flags with the user condition validator being used in the Django checks.

The other observation I have is, with a boolean condition that is True, the user condition is unnecessary unless the user condition is also required.

But the traceback suggests to me that we should be handling model errors in the checks better than we are. I'll see if I can get a fix in.

@moaxey
Copy link
Author

moaxey commented Mar 8, 2022

Thankyou @willbarton

@cristobalmackenzie
Copy link

cristobalmackenzie commented Oct 4, 2023

We ran into the same issue trying to deploy a new user column. Luckily our django-flags usage is very sparse and we were able to delete them, migrate and then recreate them.

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
psycopg2.errors.UndefinedColumn: column users_user.mobile_number does not exist
LINE 1: ..._user"."is_active", "users_user"."is_accounting", "users_use...
                                                             ^


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/app/manage.py", line 39, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 89, in wrapped
    res = handle_func(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/core/management/commands/migrate.py", line 75, in handle
    self.check(databases=[database])
  File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 419, in check
    all_issues = checks.run_checks(
  File "/usr/local/lib/python3.10/site-packages/django/core/checks/registry.py", line 76, in run_checks
    new_errors = check(app_configs=app_configs, databases=databases)
  File "/usr/local/lib/python3.10/site-packages/flags/checks.py", line 30, in flag_conditions_check
    condition.fn.validate(condition.value)
  File "/usr/local/lib/python3.10/site-packages/flags/conditions/validators.py", line 45, in validate_user
    UserModel.objects.get(**{UserModel.USERNAME_FIELD: value})
  File "/usr/local/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 431, in get
    num = len(clone)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 262, in __len__
    self._fetch_all()
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 1324, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 51, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 1175, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 98, in execute
    return super().execute(sql, params)
  File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 66, in execute
    return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
  File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 75, in _execute_with_wrappers
    return executor(sql, params, many, context)
  File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 79, in _execute
    with self.db.wrap_database_errors:
  File "/usr/local/lib/python3.10/site-packages/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.10/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: column users_user.mobile_number does not exist
LINE 1: ..._user"."is_active", "users_user"."is_accounting", "users_use...
                                                             ^

I'll take a look to see if I can come up with a fix, though I'm not sure what the desired behaviour should be.

@cristobalmackenzie
Copy link

@willbarton I think I'd catch the django.db.utils.ProgrammingError inside the validate_user function and raise a ValidationError. In the flags_condition_check maybe use a new error code to mean that the validation couldn't run properly, and add a "There seems to be an unapplied migration in the User model" message of some sort.

If that seems like a decent solution I'll work on the PR

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

No branches or pull requests

3 participants