citusdata / django-multitenant

Python/Django support for distributed multi-tenant databases like Postgres+Citus
MIT License
708 stars 116 forks source link

Django 4.2b1 is released and some tests are failing #156

Open gurkanindibay opened 1 year ago

gurkanindibay commented 1 year ago

https://github.com/citusdata/django-multitenant/actions/runs/4262532233

apagano-vue commented 1 year ago

@gurkanindibay : I've looked into this to make sure this lib would work as expected with our version of Django (4.2.2) and it seems the failures are caused by two different issues: 

  1. In version 4.2 the django backend db logger (used in the built-in assertNumQueries helper) now logs BEGIN, COMMIT, and ROLLBACK operations (see https://docs.djangoproject.com/en/4.2/releases/4.2/#logging). This adds two additional captured queries (BEGIN+COMMIT) for all the test involving a deletion (test_delete_tenant_set, test_delete_cascade_distributed, test_delete_cascade_distributed, test_delete_cascade_reference_to_distributed, test_delete_cascade_distributed_to_reference and test_delete_tenant_set). For those tests the fix is simple: add 2 to the expected count of queries. The following also needs to be added to test_delete_tenant_set to avoid failing the assertion because of those two additional queries: 
            for query in captured_queries.captured_queries:
                if "BEGIN" == query["sql"] or "COMMIT" == query["sql"]:
                    continue
                if "tests_revenue" in query["sql"]:
                    self.assertTrue(f'"acc_id" = {account.id}' in query["sql"])
                else:
                    self.assertTrue(f'"account_id" = {account.id}' in query["sql"])
  1. Even with the fix for point 1 test_delete_cascade_distributed_to_reference still fails. A quick diff between the output of running the test with Django 4.1 and django 4.2 shows this: 

Django 4.1: 

...
E   4. SELECT "tests_project"."id" FROM "tests_project" WHERE ("tests_project"."employee_id" IN (4) AND "tests_project"."account_id" = 1)
...
E   6. SELECT "tests_project"."id" FROM "tests_project" WHERE ("tests_project"."account_id" IN (1) AND "tests_project"."account_id" = 1)
...

Django 4.2: 

...
E   4. SELECT "tests_project"."id" FROM "tests_project" WHERE ("tests_project"."account_id" IN (1) AND "tests_project"."account_id" = 1)
...

So the first SELECT statement is not happening anymore.

I would be happy to make a PR to resolve 1 but I'm afraid I am not familiar enough with the library to know what is causing 2 (is it a bug/breaking change that should be fixed or just a minor change as is point 1?), how critical it is and what should be done about it.

Let me know what you think