-
Notifications
You must be signed in to change notification settings - Fork 31
🐛⚗️ Fix error importing a pg dump of a vanilla, empty, alembic-created simcore pg DB #8166
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
base: master
Are you sure you want to change the base?
🐛⚗️ Fix error importing a pg dump of a vanilla, empty, alembic-created simcore pg DB #8166
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8166 +/- ##
==========================================
+ Coverage 88.05% 90.14% +2.08%
==========================================
Files 1921 1604 -317
Lines 74458 63500 -10958
Branches 1305 562 -743
==========================================
- Hits 65562 57239 -8323
+ Misses 8502 6113 -2389
+ Partials 394 148 -246
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mrnicegyu11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. that migration is from 2020. not sure I understand why only that one would require the public prefix??
@@ -21,7 +22,7 @@ def upgrade(): | |||
""" | |||
CREATE OR REPLACE FUNCTION check_group_uniqueness(name text, type text) RETURNS INT AS $$ | |||
BEGIN | |||
IF type = 'EVERYONE' AND (SELECT COUNT(*) FROM groups WHERE groups.type = 'EVERYONE') = 1 THEN | |||
IF type = 'EVERYONE' AND (SELECT COUNT(*) FROM public.groups WHERE groups.type = 'EVERYONE') = 1 THEN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public
? this is the default name of the schema
(SEE postgres/scripts
). Why would this be needed.
if so, why do we need it here and not in other tables?
I am not sure I can explain this better to you beyond "this is what fixed the import for me", maybe Matus knows from his background of postgres? I am myself not sure. If in doubt, we can lose this PR, let me know in case ;) |
🧪 CI InsightsHere's what we observed from your CI run for 32161dd. 🟢 All jobs passed!But CI Insights is watching 👀 |
since it is not clear to me if there is anything I can do to get this merged, I will close it as stale :--) |
I encountered this issue again with @matusdrobuliak66 trying to import the postgres from aws-master locally. This would not work because of the precise issue in this PR. Adding the BUT: One can not just modify the
@matusdrobuliak66 re-opening this and assigning to you thx! |
So just to clarify: My proposed change will not work as is, there has to come an additional migration along the specified steps that fixes the current issue in the function ;) |
|
What do these changes do?
If one creates a new, PostgreSQL database, then runs the osparc-simcore migration microservice, then dumps the postgres DB to a backup
.sql
file usingpsql
and lastly tries to import it, the import will fail withThis is due to a wrong function definition in the osparc-simcore alembic, where
groups
should bepublic.groups
.This change fixes the bug, and the .sql PG dump can then be imported.
Related issue/s
How to test
Dev-ops