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

Dependencies: Update to psycopg>=3.0 #38

Merged
merged 1 commit into from
May 30, 2024
Merged

Dependencies: Update to psycopg>=3.0 #38

merged 1 commit into from
May 30, 2024

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented May 30, 2024

No description provided.

@sphuber sphuber force-pushed the feature/psycopg-v3 branch 8 times, most recently from 747ce06 to 041fa85 Compare May 30, 2024 11:33
@codecov-commenter
Copy link

codecov-commenter commented May 30, 2024

Codecov Report

Attention: Patch coverage is 86.66667% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 95.05%. Comparing base (7a05bbc) to head (69c19b8).

Files Patch % Lines
pgsu/__init__.py 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
- Coverage   96.00%   95.05%   -0.95%     
==========================================
  Files           2        2              
  Lines         175      182       +7     
==========================================
+ Hits          168      173       +5     
- Misses          7        9       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The `database` key in the DSN was changed to `dbname` in psycopg v3. To
prevent breaking existing code, the key is automatically converted in
the `PGSU` constructor and the `_execute_su_psql` function, and  a
warning is emitted.
@sphuber sphuber force-pushed the feature/psycopg-v3 branch from 041fa85 to 69c19b8 Compare May 30, 2024 11:39
@sphuber
Copy link
Contributor Author

sphuber commented May 30, 2024

@danielhollas I have tested this in the field with aiida-core by installing from this branch. The tests now pass: aiidateam/aiida-core#6362
Do you still want to give this a look? or can I go ahead and merge and release this.

@danielhollas
Copy link
Contributor

Erm, I guess I don't remember some previous discussion about this? : 😅 Gave this a quick look and LGTM. Do I understand correctly that the changes here are backwards compatible?

@sphuber
Copy link
Contributor Author

sphuber commented May 30, 2024

Erm, I guess I don't remember some previous discussion about this?

There hasn't been :D Just that there is an open issue on aiida-core to consider moving to psycopg v3 which was released in 2021. I cannot find any mention of a timeline for the end-of-life of psycopg2 but it is bound to happen some time. Unfortunately psycopg2 and psycopg are not compatible. It may be possible to make this library compatible with both at the same time, but probably not trivial and don't feel like investing the time. sqlalchemy also treats them as two separate dialects since they are different enough.

So the plan is just to release this change as v0.3.0 which will only support psycopg. There will need to be some work done in aiida-core still to fix the engine connection string. In sqlalchemy, the default is postgres:// which is an alias for postgres+psycopg2:// which is the default. To switch to psycopg, we need to use postgres+psycopg://. But first I just wanted to make sure that the implementation runs against aiida-core, which seems to be the case. Once I have the release, I will deal with the migration part.

@sphuber sphuber merged commit 18a01a3 into master May 30, 2024
18 checks passed
@sphuber sphuber deleted the feature/psycopg-v3 branch May 30, 2024 17:27
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.

3 participants