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

Add dependency to psycopg-binary #881

Merged
merged 4 commits into from
Dec 28, 2023
Merged

Add dependency to psycopg-binary #881

merged 4 commits into from
Dec 28, 2023

Conversation

ewjoachim
Copy link
Member

Closes #880

@ashleyheath would you mind trying out this PR and letting me know if it works in your case ?

Successful PR Checklist:

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

@ewjoachim ewjoachim requested a review from a team as a code owner December 27, 2023 16:51
Copy link

github-actions bot commented Dec 27, 2023

Coverage report

The coverage rate went from 99.12% to 99.13% ⬆️
The branch rate is 98%.

100% of new lines are covered.

Diff Coverage details (click to unfold)

procrastinate/psycopg_connector.py

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

procrastinate/utils.py

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

@ashleyheath
Copy link
Contributor

@ewjoachim, have spent a while today trying to apply these into a reproducable project but haven't made great progress. Am afraid I probably won't get any more time to look at it this week but the code changes in the PR look good to me regardless so am happy for you to merge them in as-is if you're happy with them (and if so, I can quickly pick up the new release and verify that in our build). Otherwise I can try pick this up again next week.

@ewjoachim
Copy link
Member Author

Sorry for the time lost. Let's merge.

@ewjoachim ewjoachim merged commit 4b096a1 into main Dec 28, 2023
6 checks passed
@ewjoachim ewjoachim deleted the psycopg-binary branch December 28, 2023 13:02
@ashleyheath
Copy link
Contributor

Have pulled the new release into our build pipeline and can confirm it fixes the original issue.

@ewjoachim
Copy link
Member Author

Perfect!

I'm delighted that you're taking interest into contributing to the project. As you probably saw in other PRs, I'm partly in the middle of redoing some parts (especially around the sync/async situation which is currently subpar).

Would you like to be part of the effort of trying out the pre-release when this comes out?

It might include switching to Psycopg3. It looks like I'll never be able to provide a good UX with psycopg2 nor aiopg, but I'm still wondering if I should leave the psycopg2 and aiopg connectors around with a disclaimer that they'll likely cause issues for non-purely async projects, or remove it and make it harder for existing users, but better in the long run)

@turicas
Copy link
Contributor

turicas commented Dec 28, 2023

@ewjoachim how do you plan to support Django when moving to psycopg3? As far as I know, only Django 4.2+ supports psycopg3 DB backend and it could be difficult to deal with Django using psycopg2 and procrastinate using psycopg3 in the same project (there are a lot of software out there using an earlier version of Django).

@ewjoachim
Copy link
Member Author

it could be difficult to deal with Django using psycopg2 and procrastinate using psycopg3 in the same project

I'm not sure I understand how, but I'd love to discuss this. Let's do that here!

@ashleyheath
Copy link
Contributor

Would you like to be part of the effort of trying out the pre-release when this comes out?

Yep; very happy to! Will pick up on the discussion thread if/when I have comments to add

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.

Missing psycopg 'binary' implementation extra since 0.30.0?
3 participants