-
Notifications
You must be signed in to change notification settings - Fork 348
Update the docs when overriding django_db_setup #1190
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
base: main
Are you sure you want to change the base?
Conversation
@bluetech thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a review -- I need to look into it more. Just a couple of typos.
Co-authored-by: Ran Benita <[email protected]>
@bluetech thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Updates documentation examples for overriding the django_db_setup
fixture to correctly use the request
parameter and call the original fixture via request.getfixturevalue()
. This addresses reported issues where the previous examples would not work properly.
- Adds
request
parameter todjango_db_setup
fixture signatures - Changes from bare
yield
toyield request.getfixturevalue("django_db_setup")
- Updates database configuration example to use individual key assignment instead of dictionary replacement
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
from django.conf import settings | ||
|
||
settings.DATABASES['default']['NAME'] = 'the_copied_db' | ||
|
||
run_sql('DROP DATABASE IF EXISTS the_copied_db') | ||
run_sql('CREATE DATABASE the_copied_db TEMPLATE the_source_db') | ||
|
||
yield | ||
yield request.getfixturevalue("django_db_setup") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates a circular dependency where django_db_setup
tries to get its own fixture value. The correct approach would be to yield from the original fixture or use a different fixture name for the override.
yield request.getfixturevalue("django_db_setup") | |
yield |
Copilot uses AI. Check for mistakes.
settings.DATABASES['default']['HOST'] = 'db.example.com' | ||
settings.DATABASES['default']['NAME'] = 'external_db' | ||
|
||
yield request.getfixturevalue("django_db_setup") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates a circular dependency where django_db_setup
tries to get its own fixture value. The correct approach would be to yield from the original fixture or use a different fixture name for the override.
yield request.getfixturevalue("django_db_setup") | |
yield from original_django_db_setup(request) |
Copilot uses AI. Check for mistakes.
Putting it into draft becuase its been so long, and copilot is making me doubt myself. Im going to make a test.. when i have time to prove this out.. |
Fixes issue: #1183 #1131
See: #1183 (comment)