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

Adds support for psycopg version 3 #78

Merged
merged 2 commits into from
Sep 9, 2024

Conversation

romank0
Copy link
Contributor

@romank0 romank0 commented Aug 10, 2024

This solves #56 as in adds support of psycopg version 3.

It creates a wrapper around psycopg v3 connection that mimics the behaviour of the v2 API.

This also adds python 11, python 12 and django5 environment to tox.

I haven't changed dev dependencies in this PR and they are still locked on django4 and psycopg2.

@PaulGilmartin
Copy link
Owner

@romank0 Thanks again for your contributions. This one is a great help. I'll review this within the next week and hopefully get it merged!

@romank0
Copy link
Contributor Author

romank0 commented Sep 9, 2024

I did test this and it works in general and this is used on production for a couple weeks now. However I'm not sure this is the best way to implement psycopg3 support in terms of efficiency. Specifically I don't like the roundtrip to server to fetch notifications. In psycopg2 the notifications fetching is a client side operation but I didn't have much knowledge on the psycopg3 to do all this client side.

But now I'm working on a fix to workaround a nasty races bug in psycopg2 (and apparentrly in psycopg3) and I have better understanding on the pqlib and inner workings of both psycopg2 and psycopg3. As part of that work I might change this implementation a bit.

@PaulGilmartin
Copy link
Owner

@romank0 Thanks for the extra info. When I tried to perform the upgrade I couldn't get it to work without incurring an extra query either. I agree that is not optimal. However, as this change does not force the user to incur that extra query unless they used v3, I think we should merge it. If you have any break throughs or need any help in terms of removing the extra query, let me know. I'll also make it clearer in the docs and readme that psycopg3 needs an extra query when polling.

@PaulGilmartin PaulGilmartin merged commit 56b8ff5 into PaulGilmartin:master Sep 9, 2024
1 check passed
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