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

fix: stabilize makemigrations when SITE_ID != 1 #34787

Merged
merged 1 commit into from May 22, 2024

Conversation

kdmccormick
Copy link
Member

Some models in third_party_auth used settings.SITE_ID as a field default, which caused Django to say migrations were out of sync whenever settings.SITE_ID happened to be anything other than 1 for any developer:

Your models in app(s): 'third_party_auth' have changes that are not
yet reflected in a migration, and so won't be applied. Run
'manage.py makemigrations' to make new migrations, and then re-run
'manage.py migrate' to apply them.

This could easily happen if a developer is testing out site configuration or site-specific theming and ends up with a SITE_ID other than 1.

The fix, inspired by a StackOverflow answer [1], is to simply create a wrapper function for the dynamic default value. The wrapper function, rather than the current value of SITE_ID, will be serialized to the migraiton file.

This commit includes a migration file, but from a database perspective, the migration is a no-op.

[1] https://stackoverflow.com/a/12654998

Some models in third_party_auth used settings.SITE_ID as a field
default, which caused Django to say migrations were out of sync whenever
settings.SITE_ID happened to be anything other than 1 for any developer:

    Your models in app(s): 'third_party_auth' have changes that are not
    yet reflected in a migration, and so won't be applied. Run
    'manage.py makemigrations' to make new migrations, and then re-run
    'manage.py migrate' to apply them.

This could easily happen if a developer is testing out site
configuration or site-specific theming and ends up with a SITE_ID other
than 1.

The fix, inspired by a StackOverflow answer [1], is to simply create
a wrapper function for the dynamic default value. The wrapper function,
rather than the current value of SITE_ID, will be serialized to the
migraiton file.

This commit includes a migration file, but from a database perspective,
the migration is a no-op.

[1] https://stackoverflow.com/a/12654998
@kdmccormick kdmccormick merged commit ccf2b75 into openedx:master May 22, 2024
47 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/tpa-migration branch May 22, 2024 17:52
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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

Successfully merging this pull request may close these issues.

None yet

3 participants