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!: Support Django 4.2 #865

Merged
merged 15 commits into from
Apr 25, 2024
Merged

feat!: Support Django 4.2 #865

merged 15 commits into from
Apr 25, 2024

Conversation

ankiaga
Copy link
Contributor

@ankiaga ankiaga commented Apr 12, 2024

Breaking changes

  • Removes support for Django 2.2
  • Transaction handling in Django 4.2 is different than in Django 3.2:
    • In Django 3.2: When Django tries to manually start a transaction while the dbapi connection is in AutoCommit=True mode, the Spanner Django provider would not start a transaction.
    • In Django 4.2: When Django wants to manually start a transaction while the dbapi connection is in AutoCommit=True mode, the Spanner Django provider will start a transaction.

Release notes

  1. Adds support for Django 4.2
  2. Removes support for Django 2.2
  3. Only Python versions 3.8 and higher are supported
  4. Adds support for transactions in AutoCommit=True mode by default for V4.2. Transactions in AutoCommit=True mode are by default not supported in prior versions (i.e. V3.2).
    • To enable manual transactions in AutoCommit=True mode in V3.2, please set ALLOW_TRANSACTIONS_IN_AUTO_COMMIT=True your settings.py file.
    • To disable transactions in AutoCommit=True mode in V4.2, please set ALLOW_TRANSACTIONS_IN_AUTO_COMMIT=False in your settings.py file.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner-django API. label Apr 12, 2024
@ankiaga ankiaga requested a review from olavloite April 12, 2024 05:32
)
from django.db.models import JSONField
USING_DJANGO_4 = False
if django.VERSION[:2] == (4, 2):
Copy link

@jaz-la jaz-la Apr 15, 2024

Choose a reason for hiding this comment

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

as commented in this draft:
#860 (comment)

Currently Django 5 is released, since you are already working on it, can this field be override with a system environment, and make it something like:
if django.VERSION[:2] == (4, 2) or os.getenv('USING_DJANGO_4', 'false') == 'true'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry we cant do that as we are not sure how will Django 5 behave with this code.
Please feel free to create a fork and make changes

Copy link

Choose a reason for hiding this comment

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

Can I second @jaz-la's request? If we can appropriately set expectations with the env car, what's the harm? I am currently using a fork that is hard to maintain with Django 5, and this PR includes everything that I have needed for Django 5 so far. Please?

Thanks again for all of your work.

django_spanner/__init__.py Outdated Show resolved Hide resolved
django_spanner/__init__.py Outdated Show resolved Hide resolved
@jaz-la
Copy link

jaz-la commented Apr 15, 2024

thanks a lot for working on it, left a few comments.
I'm hoping that we can also have a way to bypass the version check and fall into Django 4 logic with Django 5 at our own risk.
Another suggestion is to add a summary in the PR description so new developers can understand the actual difference between django 3 vs 4 without digging into the code.

@ankiaga ankiaga changed the title feat: Changes for Django 4.2 feat!: Changes for Django 4.2 Apr 16, 2024
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
django_spanner/__init__.py Show resolved Hide resolved
django_spanner/base.py Outdated Show resolved Hide resolved
django_spanner/base.py Show resolved Hide resolved
django_spanner/compiler.py Show resolved Hide resolved
django_test_suite_4.2.sh Show resolved Hide resolved
noxfile.py Show resolved Hide resolved
tests/unit/django_spanner/test_operations.py Outdated Show resolved Hide resolved
@ankiaga ankiaga added the release-please:force-run To run release-please label Apr 17, 2024
@release-please release-please bot removed the release-please:force-run To run release-please label Apr 17, 2024
Copy link

@jaz-la jaz-la left a comment

Choose a reason for hiding this comment

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

thank you for working on it, I have been wanting to upgrade my django version for a long time and this is the last library that blocks me from doing so.

@olavloite olavloite changed the title feat!: Changes for Django 4.2 feat!: Support Django 4.2 Apr 25, 2024
@ankiaga ankiaga merged commit 75cd9bc into main Apr 25, 2024
32 checks passed
@ankiaga ankiaga deleted the django-4.2 branch April 25, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/python-spanner-django API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants