feat: migrate Django 5.2 upgrade and 3.2/4.2 deprecations by sinhasubham · Pull Request #16624 · googleapis/google-cloud-python

sinhasubham · 2026-04-13T09:07:38Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

gemini-code-assist

Code Review

This pull request updates the Spanner database backend to support Django 5.2 and Python 3.10+, while removing legacy support for Django 3.2 and 4.2. Key changes include the implementation of a global client cache to prevent initialization crashes, support for JSONArray, GeneratedField, and db_default, and a retry mechanism for flushing tables with foreign key constraints. Feedback highlights several critical issues: the use of a 32-bit mask for primary key generation significantly reduces the key space, and making GOOGLE_CLOUD_PROJECT mandatory is a breaking change for local development. Additionally, the package version was incorrectly decremented, and a minor version bump was recommended for this release. Finally, using WHERE 1=1 was suggested as a more idiomatic alternative to WHERE true for flush operations.

…bsolete workflows

surbhigarg92 · 2026-04-20T07:07:21Z


+# Global cache for Spanner client to prevent multiple initializations
+# which can cause OpenTelemetry 'MeterProvider override' crashes.
+_SPANNER_CLIENT_CACHE = None


I dont understand why do we need this, ideally our Spanner client library should handle the override crash ?

surbhigarg92 · 2026-04-20T07:09:46Z

+            "project": os.environ.get("GOOGLE_CLOUD_PROJECT")
+            or self.settings_dict.get("project")
+            or self.settings_dict.get("PROJECT")
+            or "test-project",


This is being done in multiple places, better to extract in "init" method once and use it everywhere.

Also why do we support self.settings_dict.get("project") and self.settings_dict.get("PROJECT") now ?

surbhigarg92 · 2026-04-20T07:30:17Z

+        conn_params.pop("instance", None)
+        conn_params.pop("instance_id", None)
+        conn_params.pop("client", None)


Why this change is required ?
We are not using instance_id or client anywhere. We are only using instance.instance_id or instance._client

surbhigarg92 · 2026-04-20T08:13:21Z

+        # This method copies the complete code of this overridden method from
+        # Django core and modify it for Spanner by adding one line


This is duplicate comment

surbhigarg92 · 2026-04-20T08:15:57Z

+                except AttributeError:
+                    # The test method might not exist in this version of Django.
+                    pass


Why do we need this, we have already added an explicit list of skip tests

surbhigarg92 · 2026-04-20T08:23:54Z

 # https://developers.google.com/open-source/licenses/bsd

-__version__ = "4.0.3"
+__version__ = "4.1.0"


Why are we modifying this version manually ? shouldn't this be updated automatically once we do the release ?

surbhigarg92 · 2026-04-20T08:27:50Z

        self.assertEqual(
            sql_compiled,
-            "SELECT tests_report.name FROM tests_report ORDER BY "
+            "SELECT tests_report.name AS name FROM tests_report ORDER BY "


For my knowledge why this change is required ?

surbhigarg92 · 2026-04-20T08:32:12Z


 UNIT_TEST_PYTHON_VERSIONS = [
-    "3.8",
    "3.9",


I thought we received 3.9 support as well, as we removed 3.9 integration test suite file

surbhigarg92 · 2026-04-20T08:32:53Z

+    if session.python == "3.9":
+        session.skip("Python 3.9 is not supported for Django 5.2 tests")


Lets remove 3.9 from the list then ?

surbhigarg92 · 2026-04-20T08:35:21Z

 # https://developers.google.com/open-source/licenses/bsd

-__version__ = "4.0.3"
+__version__ = "4.1.0"


Also this should go as a Major version release, since we are deprecating older vesion of Django.

PR title should be feat! to trigger a major version

sakthivelmanii · 2026-04-20T06:04:44Z

-from django.db import DEFAULT_DB_ALIAS
-from django.db.models.fields import (
+
+from django.db import DEFAULT_DB_ALIAS  # noqa: E402


why are you suppressing linting errors?

sakthivelmanii · 2026-04-20T06:06:28Z


 def gen_rand_int64():
    # Credit to https://stackoverflow.com/a/3530326.
+    # Use 32-bit integer for Emulator compatibility (High-bit issues observed).


Is this a valid comment?

sakthivelmanii · 2026-04-20T06:07:55Z

-                    method_name,
-                    skip("unsupported by Spanner")(method),
-                )
+                try:


Make you sure you clean up the method. Not suppress the error.

sakthivelmanii · 2026-04-20T06:09:42Z

            "constraint": self.sql_check_constraint % {"check": check},
        }

-    def _unique_sql(


why are we removing it?

sakthivelmanii · 2026-04-20T08:36:49Z

@@ -28,7 +28,6 @@
 DEFAULT_PYTHON_VERSION = "3.14"

 UNIT_TEST_PYTHON_VERSIONS = [


Remove support for 3.8 and 3.9

sakthivelmanii · 2026-04-20T08:38:21Z

        else:
            return []

+    def execute_sql_flush(self, sql_list):


this looks so bruteforce. is there any other better approach?

sakthivelmanii · 2026-04-20T08:40:05Z

    # Spanner does not support expression indexes
    # example: CREATE INDEX index_name ON table (LOWER(column_name))
    supports_expression_indexes = False
+    supports_stored_generated_columns = True

    # Django tests that aren't supported by Spanner.
    skip_tests = (


We need to group them by cause. Otherwise we will not know if we are doing it correctly or not.

surbhigarg92 · 2026-04-20T08:44:07Z

+                        try:
+                            cursor.execute(sql)
+                            progress = True
+                        except Exception as e:


catch more specific exception ?

sinhasubham changed the title ~~Django upgrade 5 2~~ feat: migrate Django 5.2 upgrade and 3.2/4.2 deprecations Apr 13, 2026

gemini-code-assist Bot reviewed Apr 13, 2026

View reviewed changes

sinhasubham added 2 commits April 14, 2026 22:29

feat: migrate Django 5.2 upgrade and 3.2/4.2 deprecations

d1ae45b

feat(django-spanner): Stabilize system tests for Django 5.2 migration

c54a68d

sinhasubham force-pushed the django_upgrade_5_2 branch 4 times, most recently from 2b71c2a to f6098f1 Compare April 16, 2026 09:17

chore(django-spanner): stabilize Django 5.2 system tests and remove o…

4fc923b

…bsolete workflows

sinhasubham force-pushed the django_upgrade_5_2 branch from f6098f1 to 4fc923b Compare April 16, 2026 10:03

sinhasubham requested a review from surbhigarg92 April 20, 2026 05:57

sinhasubham marked this pull request as ready for review April 20, 2026 05:57

sinhasubham requested review from a team as code owners April 20, 2026 05:57

surbhigarg92 reviewed Apr 20, 2026

View reviewed changes

sakthivelmanii reviewed Apr 20, 2026

View reviewed changes

surbhigarg92 reviewed Apr 20, 2026

View reviewed changes

surbhigarg92 requested changes Apr 20, 2026

View reviewed changes

parthea mentioned this pull request Apr 21, 2026

chore: complete migration of django-google-spanner to librarian #16748

Draft

		# This method copies the complete code of this overridden method from
		# Django core and modify it for Spanner by adding one line

		if session.python == "3.9":
		session.skip("Python 3.9 is not supported for Django 5.2 tests")

		@@ -28,7 +28,6 @@
		DEFAULT_PYTHON_VERSION = "3.14"

		UNIT_TEST_PYTHON_VERSIONS = [

Conversation

sinhasubham commented Apr 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants