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.

Sign up for GitHub

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

Jump to bottom

Fix support for autofields in non-default databases #784

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix support for autofields in non-default databases #784

wants to merge 2 commits into from

Conversation

untitaker
Copy link

Fix #783

Also refactored code a bit such that version detection is no longer used
to determine whether BigAutoField exists. BigAutoField exists in Django
versions older than 3.0, and the way this is currently monkeypatched
prevents us from using any BigAutoField in Spanner with Django 2.2.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner-django API. label Aug 9, 2022
@google-cla
Copy link

google-cla bot commented Aug 9, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@untitaker
fix: Support for autofields in non-default databases
e02e390 
Fix googleapis#783

Also refactored code a bit such that version detection is no longer used
to determine whether BigAutoField exists. BigAutoField exists in Django
versions older than 3.0, and the way this is currently monkeypatched
prevents us from using any BigAutoField in Spanner with Django 2.2.
@untitaker
Copy link
Author

Turns out, neither commit in this PR is sufficient. The django migration recorder can run in any DB, so router.db_for_write is useless.

Need to patch the querybuilder to succeed IMO.

@untitaker untitaker mentioned this pull request Aug 9, 2022
Closed
@untitaker
Copy link
Author

untitaker commented Aug 9, 2022 •
edited

I am convinced that the validate_autopk_value function is the right place to hook, as it does not require monkeypatching and can be defined per db backend, but that function will not be called unless there is a value for the autofield. Which would bring us back to patching models/fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: spanner Issues related to the googleapis/python-spanner-django API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

django-spanner's autofield generation does not work for non-default spanner DBs
1 participant
@untitaker