Skip to content

Fix LiveServer for in-memory SQLite database on Django >= 3. #947

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

Closed
wants to merge 5 commits into from
Closed

Fix LiveServer for in-memory SQLite database on Django >= 3. #947

wants to merge 5 commits into from

Conversation

icemac
Copy link

@icemac icemac commented Sep 29, 2021

allow_thread_sharing is no longer writeable since Django 3.0a1.

`allow_thread_sharing` is no longer writeable since Django 3.0a1.
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks @icemac, we should definitely fix this. I left a couple of comments.

@@ -24,7 +24,10 @@ def __init__(self, addr: str) -> None:
and conn.settings_dict["NAME"] == ":memory:"
):
# Explicitly enable thread-shareability for this connection
conn.allow_thread_sharing = True
try:
conn.inc_thread_sharing()
Copy link
Member

Choose a reason for hiding this comment

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

We also need to do the corresponding dec_thread_sharing in stop(). Django's LiveServerTestCase does it here: https://github.com/django/django/blob/3ff7f6cf07a722635d690785c31ac89484134bee/django/test/testcases.py#L1578

This is a pre-existing issue -- the previous code neglected set allow_thread_sharing back as well, but this is a good opportunity to fix it.

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure id my copy-paste solution is correct, feel free to change my code.

@icemac
Copy link
Author

icemac commented Dec 1, 2021

I am not planning to fix the coverage problems. This is too much for a drive-by contribution.

@bluetech
Copy link
Member

bluetech commented Dec 1, 2021

Thanks @icemac, merged in #973. BTW, turns out it was backported to Django 2.2 so no need for the version checks at all.

@bluetech bluetech closed this Dec 1, 2021
@icemac icemac deleted the fix-live-server-sqlite-in-memory branch December 2, 2021 07:16
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.

2 participants