-
Notifications
You must be signed in to change notification settings - Fork 7
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 migration to remove obsolete settings (facets_enabled_collection … #2204
Add migration to remove obsolete settings (facets_enabled_collection … #2204
Conversation
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.
Need to update the table this migration is targeting, see comment on PR.
conn.execute( | ||
"UPDATE integration_library_configurations set settings = settings::jsonb - 'facets_enabled_collection'" | ||
) | ||
conn.execute( | ||
"UPDATE integration_library_configurations set settings = settings::jsonb - 'facets_default_collection'" | ||
) |
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.
These settings are stored in the libraries
table, not the integration_library_configurations
table.
Minor: You can also do the update in a single update statement and without using a cast to jsonb
since the column already has that datatype.
conn.execute( | |
"UPDATE integration_library_configurations set settings = settings::jsonb - 'facets_enabled_collection'" | |
) | |
conn.execute( | |
"UPDATE integration_library_configurations set settings = settings::jsonb - 'facets_default_collection'" | |
) | |
conn.execute( | |
"UPDATE libraries set settings_dict = " | |
"settings_dict - array['facets_enabled_collection', 'facets_default_collection']" | |
) |
@jonathangreen : I'll upgrade the migration target once I've merged in the new events PR which also has a migration. |
b446928
to
8de86bc
Compare
8de86bc
to
336ec92
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2204 +/- ##
=======================================
Coverage 91.09% 91.09%
=======================================
Files 363 363
Lines 41201 41201
Branches 8827 8827
=======================================
Hits 37531 37531
Misses 2406 2406
Partials 1264 1264 ☔ View full report in Codecov by Sentry. |
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.
Looks good
…and facets_default_collection).
Description
This is a follow-up PR to clean-up obsolete settings removed in #2182
Motivation and Context
How Has This Been Tested?
Checklist