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

Feat: check for existing superuser #1851

Closed
wants to merge 2 commits into from
Closed

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Jan 10, 2024

If ./bin/init.sh has already been run and then is run again with DB_RESET set to false, it runs into an error related to the superuser:

Log snippet
2024-01-08T23:46:28.287623934Z + bin/init.sh
2024-01-08T23:46:28.471919157Z + DB_DIR=./data
2024-01-08T23:46:28.471943758Z + DB_FILE=./data/django.db
2024-01-08T23:46:28.471947658Z + DB_RESET=false
2024-01-08T23:46:28.471950858Z + [[ false = true ]]
2024-01-08T23:46:28.471954158Z + python manage.py migrate
2024-01-08T23:46:35.050203435Z [08/Jan/2024 23:46:34] INFO benefits.oauth.client:51 Registering OAuth clients
2024-01-08T23:46:35.050323336Z /home/calitp/.local/lib/python3.11/site-packages/django/db/backends/utils.py:98: RuntimeWarning: Accessing the database during app initialization is discouraged. To fix this warning, avoid executing queries in AppConfig.ready() or when your app modules are imported.
2024-01-08T23:46:35.050333936Z   warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning)
2024-01-08T23:46:35.050337836Z [08/Jan/2024 23:46:34] DEBUG benefits.oauth.client:56 Registering OAuth client: cdt-logingov-ial2
2024-01-08T23:46:35.050341436Z [08/Jan/2024 23:46:34] DEBUG benefits.oauth.client:56 Registering OAuth client: cdt-vagov
2024-01-08T23:46:35.050345036Z [08/Jan/2024 23:46:34] DEBUG benefits.core.admin:21 Register EligibilityType
2024-01-08T23:46:35.050359536Z [08/Jan/2024 23:46:34] DEBUG benefits.core.admin:21 Register EligibilityVerifier
2024-01-08T23:46:35.050363636Z [08/Jan/2024 23:46:34] DEBUG benefits.core.admin:21 Register PaymentProcessor
2024-01-08T23:46:35.050367236Z [08/Jan/2024 23:46:34] DEBUG benefits.core.admin:21 Register PemData
2024-01-08T23:46:35.050370936Z [08/Jan/2024 23:46:34] DEBUG benefits.core.admin:21 Register TransitAgency
2024-01-08T23:46:35.081545016Z [08/Jan/2024 23:46:35] DEBUG benefits.core.analytics:111 Initialize Client for https://api2.amplitude.com/2/httpapi
2024-01-08T23:46:35.099570579Z [08/Jan/2024 23:46:35] DEBUG benefits.core.urls:41 Register path converter: TransitAgencyPathConverter
2024-01-08T23:46:35.235399800Z [08/Jan/2024 23:46:35] DEBUG benefits.urls:39 Register admin urls
2024-01-08T23:46:35.387978272Z Operations to perform:
2024-01-08T23:46:35.456002284Z   Apply all migrations: admin, auth, contenttypes, core, sessions
2024-01-08T23:46:35.456107285Z Running migrations:
2024-01-08T23:46:35.456122585Z   No migrations to apply.
2024-01-08T23:46:36.173646737Z + [[ true = true ]]
2024-01-08T23:46:36.173686937Z + python manage.py createsuperuser --no-input
2024-01-08T23:46:40.238376879Z [08/Jan/2024 23:46:40] INFO benefits.oauth.client:51 Registering OAuth clients
2024-01-08T23:46:40.265089419Z /home/calitp/.local/lib/python3.11/site-packages/django/db/backends/utils.py:98: RuntimeWarning: Accessing the database during app initialization is discouraged. To fix this warning, avoid executing queries in AppConfig.ready() or when your app modules are imported.
2024-01-08T23:46:40.265196120Z   warnings.warn(self.APPS_NOT_READY_WARNING_MSG, category=RuntimeWarning)
2024-01-08T23:46:40.273719496Z [08/Jan/2024 23:46:40] DEBUG benefits.oauth.client:56 Registering OAuth client: cdt-logingov-ial2
2024-01-08T23:46:40.276376020Z [08/Jan/2024 23:46:40] DEBUG benefits.oauth.client:56 Registering OAuth client: cdt-vagov
2024-01-08T23:46:40.278285437Z [08/Jan/2024 23:46:40] DEBUG benefits.core.admin:21 Register EligibilityType
2024-01-08T23:46:40.279116645Z [08/Jan/2024 23:46:40] DEBUG benefits.core.admin:21 Register EligibilityVerifier
2024-01-08T23:46:40.285648603Z [08/Jan/2024 23:46:40] DEBUG benefits.core.admin:21 Register PaymentProcessor
2024-01-08T23:46:40.286111908Z [08/Jan/2024 23:46:40] DEBUG benefits.core.admin:21 Register PemData
2024-01-08T23:46:40.307941404Z [08/Jan/2024 23:46:40] DEBUG benefits.core.admin:21 Register TransitAgency
2024-01-08T23:46:40.383256781Z [08/Jan/2024 23:46:40] DEBUG benefits.core.analytics:111 Initialize Client for https://api2.amplitude.com/2/httpapi
2024-01-08T23:46:40.385759203Z [08/Jan/2024 23:46:40] DEBUG benefits.core.urls:41 Register path converter: TransitAgencyPathConverter
2024-01-08T23:46:40.522614133Z [08/Jan/2024 23:46:40] DEBUG benefits.urls:39 Register admin urls
2024-01-08T23:46:40.720922115Z CommandError: Error: That username is already taken.

This PR checks if the superuser already exists and if not, creates it using User.create_superuser.

@angela-tran angela-tran added this to the Admin tool: foundation milestone Jan 10, 2024
@angela-tran angela-tran self-assigned this Jan 10, 2024
@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Jan 10, 2024
Copy link

github-actions bot commented Jan 10, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits
  superuser.py 1-23
Project Total  

This report was generated by python-coverage-comment-action

@@ -23,7 +23,7 @@ python manage.py migrate
# check DJANGO_ADMIN = true, default to false if empty or unset

if [[ ${DJANGO_ADMIN:-false} = true ]]; then
python manage.py createsuperuser --no-input
cat benefits/superuser.py | python manage.py shell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@angela-tran angela-tran marked this pull request as ready for review January 11, 2024 00:21
@angela-tran angela-tran requested a review from a team as a code owner January 11, 2024 00:21
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much more elegant than the pure shell approach 👍

Since we are introducing new Python code though, I'd like to see some test coverage, e.g. for each of the conditionals.


User = get_user_model() # get the currently active user model

username = os.environ.get("DJANGO_SUPERUSER_USERNAME")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the env vars are undefined, os.environ.get(....) hides this fact and the code continues along, only blowing up later.

I think we can use os.environ[...] here since these are all required values.

raise RuntimeError("A user already exists with DJANGO_SUPERUSER_NAME as the username")
else:
email = os.environ.get("DJANGO_SUPERUSER_EMAIL")
password = os.environ.get("DJANGO_SUPERUSER_PASSWORD")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about os.environ[...] for these two.

@angela-tran
Copy link
Member Author

Closing in favor of #1856

@angela-tran angela-tran deleted the fix/check-superuser-exists branch January 19, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants