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

Staging server might have charts with variables in config that don't exist in variables table #3501

Open
1 of 3 tasks
Marigold opened this issue Nov 6, 2024 · 0 comments
Open
1 of 3 tasks
Assignees
Labels
bug Something isn't working priority 2 - important

Comments

@Marigold
Copy link
Collaborator

Marigold commented Nov 6, 2024

Problem

We found a chart on the staging server with a variable ID in its config.dimensions that didn't exist in the variables table. This caused chart-sync to fail with an error about a non-existent variable. The missing variable actually existed but had a different id. The issue likely unfolded as follows:

  1. A chart and variable were happily coexisting in MySQL.
  2. We edited the variable's shortName, and ETL deleted the original variable and created a new one with a different ID.
  3. This shouldn't have happened! If ETL intends to delete a variable, it first checks the chart_dimensions table to ensure the variable isn't in use (and if it is, ETL raises an error).
    • Either the variable wasn't present in chart_dimensions, or there's a race condition when updating data or charts.

It's worth adding that this happened on a staging server that had some issues already.

Workaround

I resolved the issue by removing the problematic variable from the chart and replacing it with the existing one.

Impact

Since chart-sync is a crucial part of the flow now, it's a severe blocker when this comes up, although it is only coming up rarely now.

Technical notes

  • We edit variables straight in MySQL, but chart edits happen through an API
  • MySQL is meant to prevent variable deletion through foreign key constraints via chart_dimensions
    • But in this case there was no chart dimension row
    • This could be caused if the API fails and was not using a transaction when writing the config + the chart dimension rows

TODO

  • Improve error message
  • Try to replicate this on cherry blossom
  • Check whether owid-grapher is doing all its API operations on charts within transactions

Appendix

Error from chart-sync

Traceback (most recent call last):
--
  | File "/home/owid/etl/.venv/bin/etl", line 8, in <module>
  | sys.exit(cli())
  | File "/home/owid/etl/.venv/lib/python3.10/site-packages/rich_click/rich_command.py", line 367, in __call__
  | return super().__call__(*args, **kwargs)
  | File "/home/owid/etl/.venv/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
  | return self.main(*args, **kwargs)
  | File "/home/owid/etl/.venv/lib/python3.10/site-packages/rich_click/rich_command.py", line 152, in main
  | rv = self.invoke(ctx)
  | File "/home/owid/etl/.venv/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
  | return _process_result(sub_ctx.command.invoke(sub_ctx))
  | File "/home/owid/etl/.venv/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
  | return ctx.invoke(self.callback, **ctx.params)
  | File "/home/owid/etl/.venv/lib/python3.10/site-packages/click/core.py", line 783, in invoke
  | return __callback(*args, **kwargs)
  | File "/home/owid/etl/apps/chart_sync/cli.py", line 158, in cli
  | migrated_config = diff.source_chart.migrate_config(source_session, target_session)
  | File "/home/owid/etl/etl/grapher_model.py", line 491, in migrate_config
  | config = _remap_variable_ids(config, remap_ids)
  | File "/home/owid/etl/etl/grapher_model.py", line 1858, in _remap_variable_ids
  | out[k] = _remap_variable_ids(v, remap_ids)
  | File "/home/owid/etl/etl/grapher_model.py", line 1861, in _remap_variable_ids
  | return [_remap_variable_ids(item, remap_ids) for item in config]
  | File "/home/owid/etl/etl/grapher_model.py", line 1861, in <listcomp>
  | return [_remap_variable_ids(item, remap_ids) for item in config]
  | File "/home/owid/etl/etl/grapher_model.py", line 1846, in _remap_variable_ids
  | out[k] = remap_ids[int(v)]
  | KeyError: 997310
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority 2 - important
Projects
None yet
Development

No branches or pull requests

2 participants