🐛 FIX: CREATE DATABASE with psycopg2 v2.9 #27
Conversation
Codecov Report
@@ Coverage Diff @@
## master #27 +/- ##
==========================================
+ Coverage 95.83% 95.90% +0.07%
==========================================
Files 2 2
Lines 168 171 +3
==========================================
+ Hits 161 164 +3
Misses 7 7
Continue to review full report at Codecov.
|
|
should probably drop ububtu-1604 now, as it looks like the containers are no longer available, |
There was a problem hiding this comment.
thanks @chrisjsewell !
so the issue is just that the connection needs to be closed?
from looking at the code it's not so easy to see what the actual difference is
| @@ -97,9 +91,9 @@ jobs: | |||
| run: | | |||
| sudo rm -f /etc/apt/sources.list.d/dotnetdev.list /etc/apt/sources.list.d/microsoft-prod.list | |||
| sudo apt update | |||
| sudo apt install postgresql-12 | |||
There was a problem hiding this comment.
is there a reason to downgrade?
There was a problem hiding this comment.
yeh for some reason 1804 apt only has v10 available (I checked locally in a docker container)
no its that you don't use a contextmanager for the connection, so that psycopg2 does not start a transaction (which only happens in v2.9). you use a try/finally instead |
| with psycopg2.connect(**dsn) as conn: | ||
| try: | ||
| conn = psycopg2.connect(**dsn) |
There was a problem hiding this comment.
the actual fix is here
There was a problem hiding this comment.
Thanks!
could you please add a comment to that effect in the code?
i.e. "don't use a context manager so that psycopg2 does not start a transaction" ?
There is no variable one can pass to the context manager to tell it not to do this, right?
There was a problem hiding this comment.
No, not according to the issue I linked to from psycopg. As it says there, the contextmanager was not actually doing anything before (in v2.8) anyway
Fixed in aiidateam/pgsu#27 fixes #5002
Fixed in aiidateam/pgsu#27 fixes #5002
Addresses aiidateam/aiida-core#5002 (database cannot be created within a transaction)
See psycopg/psycopg2#941 (comment)
You can see the fix working, going from fe7f8ee to 2a2108c (the remaining windows failures are pre-existing from #22)