Skip to content
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

Store chart configs in R2, add retrieval at /grapher/SLUG.config.json #3823

Closed
13 of 19 tasks
danyx23 opened this issue Jul 30, 2024 · 0 comments · Fixed by #3870
Closed
13 of 19 tasks

Store chart configs in R2, add retrieval at /grapher/SLUG.config.json #3823

danyx23 opened this issue Jul 30, 2024 · 0 comments · Fixed by #3870
Assignees
Labels
ops Devops related issues (stagings servers, ...) priority 2 - important

Comments

@danyx23
Copy link
Contributor

danyx23 commented Jul 30, 2024

We want to make it easy to fetch grapher configs for charts instead of the current weird baking into the HTML between //EMBEDDED_JSON strings. This is also necessary so that when we render thumbnails for the new multidimensional data pages, we can get the relevant grapher config in a sane way. (For more context refer to Notion)

This issue is about:

  • storing chart configs in R2
  • exposing them via a CF worker at /grapher/SLUG.config.json for published charts

Must do

  • Create a new R2 bucket for production chart configs
  • Create a new R2 bucket for staging chart configs (staging servers + local dev)
  • rename the existing image R2 settings to generic R2 settings
  • add new server settings for the R2 bucket path and for the grapher config url endpoint
  • Add a migration to add a SHA-1 checksum to the chart_config table for the full config so we can later quickly check if any configs in R2 are not what we expect (e.g. because upload failed or similar)
  • When saving a chart in the admin
    • save the full chart config into the R2 bucket at /configs/by-UUID/a2941546-9aa7-41a8-8959-59a918b8ba7d.json
    • if the chart is a published standalone chart, also save a copy to /grapher/by-slug/SLUG.json
    • ensure that renames/deletes of published standalone charts are correctly taken care of
  • When storing into R2, include a SHA1 hash of the config json content and store that in the DB so we can quickly check
  • Add a worker to the staging server that tries the staging bucket and if there is no file for the given staging-server/dev-env and slug, fall back to the version from prod
  • Go through the places where we rely on the embedded grapher chart configs and replace them with fetching the new grapher configs wherever possible
  • Write a tool that can check the uploaded files in R2 with their checksum against the charts in a db and make R2 match if necessary. This tool should be useful for both the initial upload and for synchronizing in case there is a problem in the future and the two stores get out of sync
  • Ensure the sync tool exists correctly when done

Can do

  • We have some strange shadowing of charts with redirects in the DB which might add confusion to this case. It might be useful to clean the db and put in some safeguards (e.g. unpublish charts if there is a redirect taking over that slug)

Before merging

  • server settings were renamed from IMAGE_HOSTING_R2_ACCESS_KEY etc to R2_ACCESS_KEY
    • make sure prod is updated to have a copy of the new env keys
    • bulk edit all staging servers to have a copy of the new env keys
    • tell all devs to update their settings AND to modify their CF bucket permissions to include owid_grapher_configs_staging
@danyx23 danyx23 self-assigned this Jul 30, 2024
@marcelgerber marcelgerber added ops Devops related issues (stagings servers, ...) and removed needs triage labels Jul 31, 2024
@danyx23 danyx23 linked a pull request Oct 4, 2024 that will close this issue
@danyx23 danyx23 closed this as completed Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ops Devops related issues (stagings servers, ...) priority 2 - important
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants