Skip user-condition validation when the auth table is missing#140
Open
arpitjain099 wants to merge 1 commit into
Open
Skip user-condition validation when the auth table is missing#140arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
validate_user queries the user table, and the system check that runs it (flag_conditions_check) only catches ValidationError. On a fresh database the system checks run before the auth tables exist (for example during the first migrate), so the query raised OperationalError / ProgrammingError and that error propagated out of the check and aborted the migration. Catch those database errors in validate_user and skip validation in that case; the username is still validated normally once the tables exist. Fixes cfpb#96 Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #96.
validate_userdoesUserModel.objects.get(...). That validator runs inside theflag_conditions_checksystem check, which only catchesValidationError. On a fresh database the system checks run before the auth tables are created (for example during the firstmigrate), so the lookup raisesOperationalError/ProgrammingError, and since the check does not catch it, the error propagates and aborts the migration. The maintainer confirmed this in the thread, and a commenter suggested catching the database error.This catches
OperationalError/ProgrammingErrorinvalidate_userand skips validation in that case. The username cannot be checked when its table does not exist yet, and it is validated normally once the tables are in place, so the only behavior change is that a missing table no longer breaksmigrate. I went with skipping rather than raising aValidationError, since raising would surface a misleading "invalid value" warning on every migrate when the value is actually fine.Added
test_user_table_missing_is_skipped, which mocks the manager lookup to raise each error and assertsvalidate_userdoes not raise. Fullflags.tests.test_conditions_validatorssuite passes (13 tests).