search

PaulGilmartin / django-pgpubsub

A distributed task processing framework for Django built on top of the Postgres NOTIFY/LISTEN protocol.
Other
245 stars 12 forks source link

Adds support for psycopg version 3 #78

Closed romank0 closed 2 months ago

romank0 commented 3 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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.