✨ automatically migrate outdated configs / TAS-623

[sophiamersmann]

Fixes #3936 (I found v3 configs in the database, so I decided to tackle this earlier rather than later)

Summary

Adds a new migrations folder to the grapher/schema directory. migrations/migrations.ts contains the actual migration functions. I wrote migrations for all breaking changes that the schema has seen so far.

The exported function migrateGrapherConfigToLatestVersion reads the config's schema version and applies a series of migration functions if necessary. If a config's schema field is missing or invalid, it throws. (This is different from the initial implementation! We used to assume a config adheres to the latest version if the schema field was missing.)

The types are set up so that if you increase the schema version of the default grapher config and forget to add a migration, TypeScript will let you know.

When the API receives a Grapher config, we try to convert it to a valid config as soon as possible so that the rest of the code can reasonably expect a config of type GrapherInterface.

To add a new migration, one would edit migrations/migrations.ts:

• Write a migration function that updates an outdated config to the next version
• Add a clause to the runMigration function that calls the new migration function

Before merging

Important

Just before merging, check if configs of more outdated versions are currently in our prod database. At the time of writing, we only have a bunch of v3 configs.

After merging

• merge https://github.com/owid/automation/pull/13

[sophiamersmann]

• ✨ automatically migrate outdated configs / TAS-623 #3967 👈
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @sophiamersmann and the rest of your teammates on Graphite

[owidbot]

Quick links (staging server):

Site	Admin	Wizard

Login: ssh owid@staging-site-migrate-outdated-configs

• Site-screenshots: https://github.com/owid/site-screenshots/compare/migrate-outdated-configs
• SVG tester: https://github.com/owid/owid-grapher-svgs/compare/migrate-outdated-configs

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2024-09-17 12:40:27 UTC
Execution time: 1.22 seconds

[notion-workspace]

Automatically migrate outdated configs

[danyx23]

This looks very good! I really like how you made the schema versions somewhat typesafe :).

I have two minor higher level conerns: my hunch is that it would be better to migrate chart configs in the saveGrapher function instead of in the individual handlers. We'd loose the ability to type saveGrapher correctly (Grapherlnterface would be a lie), but in return every time we use saveGrapher we'd get the correct migration behaviour. One codepath where this would be missing AFAICS in the current implementation is the bulk chart editor. It uses the saveGrapher function but there is no explicit migration added for that codepath. Doing this in saveGrapher seems safer?

The other concern I have is that there are quite a few partial grapher configs in the ETL that don't specify a schema and if I understand this change they would then start breaking — is that right? Or is there a simitar change in the ETL code that sets a reasonable default (like version 603 which I think is when most of the ett code was authored)?

I think it would make sense for the ETL grapher config layer update API handler to set a default schema version if none is given but also to encourage data managers to always specify a schema version or for the updater on the ETL side to specify a reasonable default.

Let me know what you think!

[danyx23]

Cute idea to do this recursively :)

[danyx23]

Oh this is very neat - I didn't know typescript could do function overloading to narrow down types. Thanks for this!

[danyx23]

Would it be very annoying to just run our migration code here? It seems that this is a bit fragile and while it will work on prod if we check first, if we just use our migration code then this could work also on old, outdated staging servers or similar if we ever wanted to use it in such a context.

[sophiamersmann]

yes, we can also just run the migration code here 👍🏻

[sophiamersmann]

@danyx23

I have two minor higher level conerns: my hunch is that it would be better to migrate chart configs in the saveGrapher function instead of in the individual handlers. We'd loose the ability to type saveGrapher correctly (Grapherlnterface would be a lie), but in return every time we use saveGrapher we'd get the correct migration behaviour. One codepath where this would be missing AFAICS in the current implementation is the bulk chart editor. It uses the saveGrapher function but there is no explicit migration added for that codepath. Doing this in saveGrapher seems safer?

Makes sense!

The other concern I have is that there are quite a few partial grapher configs in the ETL that don't specify a schema and if I understand this change they would then start breaking — is that right? Or is there a simitar change in the ETL code that sets a reasonable default (like version 603 which I think is when most of the ett code was authored)?

I think it would make sense for the ETL grapher config layer update API handler to set a default schema version if none is given but also to encourage data managers to always specify a schema version or for the updater on the ETL side to specify a reasonable default.

Yeah, I'm a bit on the fence about this. My initial implementation was forgiving (it just assumed the latest version when missing), but then I got nervous about having schemas in our system that are incorrectly versioned. Maybe the safest thing to do for missing versions is to assume v1 and then run all migrations?

[sophiamersmann]

I chatted with Mojmir, and he'll make it so that configs authored in the ETL set the $schema field, which means it's ok to return an error on our side.

[danyx23]

Nice, looks good!