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.

Sign up for GitHub

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

Jump to bottom

Add psycopg3 compat #863

Merged
merged 9 commits into from
Dec 16, 2023
Merged

Add psycopg3 compat #863

merged 9 commits into from
Dec 16, 2023

Conversation

aleksandr-shtaub
Copy link
Contributor

A naive implementation of connector for psycopg3.
Basically, it's just a modified copy of current aiopg connector.

Successful PR Checklist:

  • Tests
  • Documentation

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 26, 2023 •
edited
Loading

Coverage report

The coverage rate went from 99.33% to 99.12% ⬇️
The branch rate is 98%.

97.58% of new lines are covered.

Diff Coverage details (click to unfold)

procrastinate/init.py

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

procrastinate/exceptions.py

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

procrastinate/psycopg_connector.py

97.41% of new lines are covered (96.15% of the complete file).
Missing lines: 35, 209, 267

procrastinate/sql/init.py

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

aleksandr-shtaub
aleksandr-shtaub commented Nov 27, 2023
View reviewed changes
procrastinate/psycopg3_connector.py Outdated
return wrapped


PERCENT_PATTERN = re.compile(r"%(?![\(s])")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused. Not sure if it's needed

@ewjoachim
Copy link
Member

Hello :)

Ha, I had started doing the same thing on my side in an attempt to solve the sync/async issues (see https://github.com/procrastinate-org/procrastinate/pull/753/commits)

I'm really thinking about doing a v1 and dropping all other connectors except psycopg3.

It seems everyone is headed the same way :) No idea yet what the right approach would be though.

@aleksandr-shtaub
Copy link
Contributor Author

Ah, didn't see it, my bad :)
Searching by "psycopg3" in GitHub doesn't show this.
Anyways, it was a good exercise to learn the differences between aiopg+psycopg2 and psycopg3.

@ewjoachim
Copy link
Member

You couldn't have seen it, I had committed it but not pushed yet (I have after I saw your PR)

Here's the diff between our 2 connectors:
https://www.diffchecker.com/V89PnXHK/

@ewjoachim
Copy link
Member

I think I'm leaning towards working with your PR first, maybe backporting some of my changes, and then do the big bang where I fix sync/async and remove other connectors in a different PR.

@aleksandr-shtaub
Copy link
Contributor Author

Okay, as you prefer.
Feel free to change the PR. I'll probably be busy until the next weekend.

@ewjoachim ewjoachim marked this pull request as ready for review December 16, 2023 21:39
@ewjoachim ewjoachim requested a review from a team as a code owner December 16, 2023 21:39
@ewjoachim
Copy link
Member

ewjoachim commented Dec 16, 2023 •
edited
Loading

Given that it's (supposedly) not breaking anything, I'll merge and release. Feel free to report on issues you find once you start using it.

@ewjoachim ewjoachim merged commit 6b0105f into procrastinate-org:main Dec 16, 2023
8 checks passed
@ewjoachim
Copy link
Member

https://pypi.org/project/procrastinate/0.31.0/

@ashleyheath ashleyheath mentioned this pull request Dec 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ewjoachim ewjoachim ewjoachim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aleksandr-shtaub @ewjoachim