-
Notifications
You must be signed in to change notification settings - Fork 5
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
fix: populate DB is broken after upgrading SQLAlchemy to 2.x version #406
Conversation
@@ -40,11 +58,12 @@ def __new__(cls, *args, **kwargs): | |||
cls.instance = object.__new__(cls) | |||
return cls.instance | |||
|
|||
def __init__(self): | |||
def __init__(self, echo_sql=True): |
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.
Set the default to True
to keep the current behavior. We should consider set it to False to avoid unnecessary SQL logs.
I'm planning to add test coverage in a separate PR as this bug is blocking at leat two other issues, |
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.
LGTM added a non blocking code suggestion to improve logs! I even tested updating the location and the behaviour in the database is as expected 👍
Co-authored-by: cka-y <[email protected]>
Summary:
SQLAlchemy 2.x added support for many-to-many relations mapping. This is handy for querying the entities as it reduces the amount of independent queries, joins, and columns within the code. While updating and creating entities, it is also possible to leave most of the heavy work to the ORM. However, this comes with some restrictions on the entity types to be used as part of the relationship arrays. This PR refactored the populate script, fixing the incompatibility with 2.x version and using the create/update capabilities offered by SqlAchemy
Successful execution on DEV
Fixes #402
Expected behavior:
Populate script executes, creates, and updates without issues.
Testing tips:
On clean DB(locally)
Create a new Feed
mdb_source_id
by oneUpdate existent Feed
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.sh
to make sure you didn't break anything