search

tobymao / saq

Simple Async Queues
https://saq-py.readthedocs.io/en/latest/
MIT License
585 stars 41 forks source link

feat: `asyncpg` adapter #177

Closed cofin closed 1 month ago

cofin commented 1 month ago

This PR implements the asyncpg adapter mentioned in #172

I've left the current psycopg adapter in as a reference, but I can remove (if needed) based on feedback here.

tobymao commented 1 month ago

i just merged my commit, can you rebase?

tobymao commented 1 month ago

i can't really compare the two implementations like this, can you redo it so that you just rewrite the existing postgres adapter?

cofin commented 1 month ago

i can't really compare the two implementations like this, can you redo it so that you just rewrite the existing postgres adapter?

Yep, can do. I'll also bring in your updated changes.

cofin commented 1 month ago

I've made the updates for the group and priority settings. I think it's ready for you to take a look at this point.

tobymao commented 1 month ago

benchmarks got 8.5x slower, it needs to be fixed before this is considered,

benchmarks/simple.py saq_pg

tobymao commented 1 month ago

i'm also not a big fan of the api of asyncio compared to psycopg, looks like it's not dbapi.

given the benchmarks, i'm so far not inclined to accept it, considering lgpl v3 only means users cannot modify the source, it doesn't seem to be that detrimental, but i do understand some companies cannot adhere to it.

cofin commented 1 month ago

That's quite a surprising difference on performance. I definitely would not have expected that great of a difference.

Re: the dbapi comment, you are correct. It does not have a dbapi compatible interface. Given this difference, do you want me to spend time investigating the performance gap?

tobymao commented 1 month ago

@cofin i've been thinking about it a bit and i apologize but i don't think i have much appetite for this at the moment, so i think we can close this for now. sorry about that and thanks for the effort.