Skip to content

Commit

Permalink
🐛 Raise warning on staging server when we cannot delete variables use…
Browse files Browse the repository at this point in the history
…d in charts (#3716)

* 🐛 Raise warning on staging server when we cannot delete variables used in charts
  • Loading branch information
Marigold authored Dec 13, 2024
1 parent c45ea3d commit b50f483
Showing 1 changed file with 16 additions and 3 deletions.
19 changes: 16 additions & 3 deletions etl/grapher_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@
from apps.backport.datasync import data_metadata as dm
from apps.backport.datasync.datasync import upload_gzip_string
from apps.chart_sync.admin_api import AdminAPI
from apps.wizard.app_pages.chart_diff.chart_diff import ChartDiffsLoader
from etl import config
from etl.db import get_engine
from etl.db import get_engine, production_or_master_engine

from . import grapher_helpers as gh
from . import grapher_model as gm
Expand Down Expand Up @@ -496,8 +497,7 @@ def cleanup_ghost_variables(engine: Engine, dataset_id: int, upserted_variable_i
if rows:
rows = pd.DataFrame(rows, columns=["chartId", "variableId"])

# raise an error if on staging server
if config.ENV == "staging":
if _raise_error_for_deleted_variables(rows):
raise ValueError(f"Variables used in charts will not be deleted automatically:\n{rows}")
else:
# otherwise show a warning
Expand Down Expand Up @@ -575,6 +575,19 @@ def cleanup_ghost_variables(engine: Engine, dataset_id: int, upserted_variable_i
return True


def _raise_error_for_deleted_variables(rows: pd.DataFrame) -> bool:
"""If we run into ghost variables that are still used in charts, should we raise an error?"""
# raise an error if on staging server
if config.ENV == "staging":
# It's possible that we merged changes to ETL, but the staging server still uses old charts. In
# that case, we first check that the charts were really modified on our staging server.
modified_charts = ChartDiffsLoader(config.OWID_ENV.get_engine(), production_or_master_engine()).df
return bool(set(modified_charts.index) & set(rows.chartId))
# if not on staging, always raise an error
else:
return True


def cleanup_ghost_sources(engine: Engine, dataset_id: int, dataset_upserted_source_ids: List[int]) -> None:
"""Remove all leftover sources that didn't get upserted into DB during grapher step.
This could happen when you rename or delete sources.
Expand Down

0 comments on commit b50f483

Please sign in to comment.