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

Asyncpg Connector implementation #669

Closed
wants to merge 2 commits into from

Conversation

nwillems
Copy link

Still very very early implementation. Mostly taking the suggested converter and loosely implementing the connector interface.

Needs:

  • Error handling - steal from others?
  • Tests - copy/paste from aiopg
  • a second set of eyes, maybe even more

Closes #417

Successful PR Checklist:

  • Tests
    • (not applicable?)
  • Documentation
    • (not applicable?)

@nwillems nwillems changed the title initial draft - needs tests Asyncpg Connector implementation Aug 16, 2022
@github-actions
Copy link

Coverage report

The coverage rate went from 99% to 95% ⬇️
The branch rate is 94%.

0% of new lines are covered.

Diff Coverage details (click to unfold)

procrastinate/asyncpg_connector.py

0% of new lines are covered (0% of the complete file).

Missing lines: 1, 3, 5, 8, 15, 16, 17, 18, 19, 20, 21, 22, 26, 27, 30, 31, 32, 34, 35, 36, 38, 39, 42, 43, 71, 72, 73, 74, 75, 76, 78, 79, 80, 81, 82, 84, 85, 86, 87, 88, 89, 91, 95, 96, 97, 100, 102, 103, 104, 105, 107, 109, 110, 115, 116, 117, 118, 120, 121, 123, 125, 126, 127, 129, 130, 131, 133, 136, 137, 139, 140, 141, 142, 144, 147, 148, 150, 151, 153, 155, 162, 165, 166, 168, 169, 170

@ewjoachim
Copy link
Member

Hey there, sorry for the delay.

I haven't been very active on the project maintenance these days. It looks like you're going in the right direction. The structure of the project means that adding a new db driver should, as you saw, only result in a single new file (plus tests).

Regarding test, it would be worth writing specific tests for the connector. Also, a sencod set of brain to ensure there's no risk for SQL injection is never too much.

I wonder if it would make sense that the converter has a method that receives query and arguments and returns the new query and the new argument instead of having to do the replacement yourself outside of the converter with %.

singingwolfboy added a commit to singingwolfboy/procrastinate that referenced this pull request Oct 20, 2022
singingwolfboy added a commit to singingwolfboy/procrastinate that referenced this pull request Oct 21, 2022
@nwillems
Copy link
Author

I'm no longer working with python project on a daily basis, and hence don't have the time to finish this work. A lot has happened, sorry for the long pause. I hope someone will pick this up.

Thanks for a great project.

@nwillems nwillems closed this Nov 22, 2023
@ewjoachim
Copy link
Member

Hey, thanks for the time you spent anyway :)

I'm currently working towards a psycopg3 implementation, and I'm starting to believe it will probably be the only supported one in the future. If people want to support other connectors, They'll likely be able to do it on their own, potentially in a dedicated package.

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.

Support for asyncpg
2 participants