Skip to content

Conversation

izabala033
Copy link

Previously, _process_response used a @transaction.atomic decorator, which evaluated the database at import time. Since the database can change at runtime, this caused connection errors in certain scenarios (in my case, multitenant server that uses request host to identify database).

This PR updates _process_response to:

Use a runtime-evaluated database alias inside a with transaction.atomic(using=...) block.

Impact:

Fixes runtime database connection issues.

Ensures profiling and response processing continue to work as intended.

Copy link
Member

@albertyw albertyw left a comment

Choose a reason for hiding this comment

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

I can confirm that there are no other transaction.atomic decorators, though there are 3 other with transaction.atomics in models.py

collector = DataCollector()
collector.stop_python_profiler()
silk_request = collector.request
with transaction.atomic(using=router.db_for_write(models.SQLQuery)):
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful to add a comment here describing why this is a with block instead of a decorator. This was actually the subject of a discussion in #801 but it did not go over the possibility of the database mutating at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants