-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
🔨 Move chart configs into chart_configs table #2957
Conversation
dd4741c
to
da01cfc
Compare
@sophiamersmann this is ready for review (though not tested properly, yet). Let me know when you're ready or when you change schema. |
@Marigold the grapher pr also needs to go through review first (and I need to do another round of testing), so it won't be before next week :) |
Quick links (staging server):
Login: chart-diff: ✅No charts for review.data-diff: ❌ Found differences= Dataset garden/artificial_intelligence/2024-08-05/epoch_aggregates_affiliation
= Table epoch_aggregates_affiliation
~ Column cumulative_count (changed metadata)
- - Describes the sector where the authors of a notable AI system have their primary affiliations. The 2024 data is incomplete and was last updated 05 August 2024.
? ^^^^^^^^
+ + Describes the sector where the authors of a notable AI system have their primary affiliations. The 2024 data is incomplete and was last updated 2 September 2024.
? ^^^^^ +++++
~ Column yearly_count (changed metadata)
- - Describes the sector where the authors of a notable AI system have their primary affiliations. The 2024 data is incomplete and was last updated 05 August 2024.
? ^^^^^^^^
+ + Describes the sector where the authors of a notable AI system have their primary affiliations. The 2024 data is incomplete and was last updated 2 September 2024.
? ^^^^^ +++++
Legend: +New ~Modified -Removed =Identical Details
Hint: Run this locally with etl diff REMOTE data/ --include yourdataset --verbose --snippet Automatically updated datasets matching weekly_wildfires|excess_mortality|covid|fluid|flunet|country_profile|garden/ihme_gbd/2019/gbd_risk are not included Edited: 2024-09-03 06:50:58 UTC |
da01cfc
to
c5f2920
Compare
922cfee
to
97f2ad5
Compare
Hey @Marigold, the chart_configs PR on the Grapher side should be ready to merge soon-ish (this or next week). How do things look on the ETL side? (I'd be happy to look over this PR if you want me to) |
97f2ad5
to
7c65611
Compare
@sophiamersmann all look good, both schema renames and calling Admin endpoint. Ready whenever you are! |
7221aa0
to
14cc01a
Compare
14cc01a
to
e3c9492
Compare
## Summary Adds a `chart_configs` table to the database, adds a `configId` column to the `charts` table that points to a `chart_configs` row, and removes the `config` column. Shouldn't result in any functionality change. ## Details - All configs are copied from the `charts` table to the `chart_configs` table (for now, `patch config = full config` since inheritance hasn't been implemented yet) - Configs have a UUID, but it's not yet used (the id of the chart is still the sequential primary key of the charts table) ## Testing - [x] Bakes locally - [x] Bakes staging site Site: - [x] http://staging-site-db-chart-configs/ - [x] http://staging-site-db-chart-configs/grapher/life-expectancy - [x] http://staging-site-db-chart-configs/explorers/coronavirus-data-explorer - [x] http://staging-site-db-chart-configs/explorers/democracy - [x] http://staging-site-db-chart-configs/explorers/conflict-data - [x] http://staging-site-db-chart-configs/poverty - [x] http://staging-site-db-chart-configs/financing-education - [x] http://staging-site-db-chart-configs/part-two-how-many-people-die-from-extreme-temperatures-and-how-could-this-change-in-the-future - [x] http://staging-site-db-chart-configs/covid-hospitalizations - [x] http://staging-site-db-chart-configs/country/germany - [x] http://staging-site-db-chart-configs/energy/country/india - [x] http://staging-site-db-chart-configs/data-insights - [x] http://staging-site-db-chart-configs/collection/top-charts - [x] http://staging-site-db-chart-configs/sdgs - [x] http://staging-site-db-chart-configs/sdgs/no-poverty Admin: - [x] Chart editor: - [x] Create new chart - [x] Save as new chart - [x] Update existing chart - [x] Publish chart - [x] Unpublish chart - [x] Delete chart - [x] Bulk chart editor - [x] Chart test page - [x] Variable page (chart list) ## On the ETL side Mojmir opened a PR for changes to the ETL: owid/etl#2957 ## To do - [x] ~Is it safe to rewrite the config's id?~ I think it's okay - [x] I don't understand why we manually insert `createdAt` and `updatedAt` since the db inserts values on insert/update? - [x] Checklist for updating the db: https://www.notion.so/owid/Database-access-in-Grapher-d9db343c2bfb4ae0b14b3dec72f686c6?pvs=4#dd5b51bab6f84630839f4173b59feba2 - [x] Let Mojmir know so that he can update the ETL db schema (wait for the table structure to be signed off on) ## To do after merging - [ ] ETL: owid/etl#2957 - [ ] Datasette: owid/analytics#138 - [ ] Schema validation: owid/automation#12
TODO
from charts
andjoin charts
type
andslug
in queriesTODO before merging
Testing
chart-diff