Skip to content

Commit 8367821

Browse files
Keep backfill counts consistent with corrupt row skips
1 parent 49832f0 commit 8367821

2 files changed

Lines changed: 41 additions & 19 deletions

File tree

python_modules/dagster/dagster/_core/storage/runs/sql_run_storage.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,22 +1087,17 @@ def _filter_backfill_candidates(
10871087

10881088
def get_backfills_count(self, filters: BulkActionsFilter | None = None) -> int:
10891089
check.opt_inst_param(filters, "filters", BulkActionsFilter)
1090+
query = self._backfills_query(filters=filters)
1091+
rows = self.fetchall(query)
1092+
backfill_candidates = self._deserialize_backfill_rows(rows)
10901093
if filters and filters.tags:
10911094
# runs can have more tags than the backfill that launched them. Since we filtered tags by
10921095
# querying for runs with those tags, we need to do an additional check that the backfills
10931096
# also have the requested tags. This requires fetching the backfills from the db and filtering them
1094-
query = self._backfills_query(filters=filters)
1095-
rows = self.fetchall(query)
1096-
backfill_candidates = self._deserialize_backfill_rows(rows)
1097-
return len(
1098-
self._apply_backfill_tags_filter_to_results(backfill_candidates, filters.tags)
1097+
backfill_candidates = self._apply_backfill_tags_filter_to_results(
1098+
backfill_candidates, filters.tags
10991099
)
1100-
1101-
subquery = db_subquery(self._backfills_query(filters=filters))
1102-
query = db_select([db.func.count().label("count")]).select_from(subquery)
1103-
row = self.fetchone(query)
1104-
count = row["count"] if row else 0
1105-
return count
1100+
return len(backfill_candidates)
11061101

11071102
def get_backfill(self, backfill_id: str) -> PartitionBackfill | None:
11081103
check.str_param(backfill_id, "backfill_id")

python_modules/dagster/dagster_tests/storage_tests/utils/run_storage.py

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,14 +1450,6 @@ def test_get_backfills_skips_corrupt_backfill_rows(
14501450
backfill_timestamp=time.time(),
14511451
)
14521452
storage.add_backfill(valid_backfill)
1453-
storage.add_run(
1454-
create_dagster_run(
1455-
run_id=make_new_run_id(),
1456-
job_name="some_pipeline",
1457-
status=DagsterRunStatus.SUCCESS,
1458-
tags={"foo": "bar", BACKFILL_ID_TAG: valid_backfill.backfill_id},
1459-
)
1460-
)
14611453

14621454
with storage.connect() as conn:
14631455
conn.execute(
@@ -1518,6 +1510,41 @@ def test_get_backfills_limit_skips_corrupt_rows_without_shrinking_page(
15181510

15191511
assert "Skipping backfill corrupt because it could not be deserialized." in caplog.text
15201512

1513+
def test_get_backfills_count_skips_corrupt_rows(
1514+
self, storage: RunStorage, caplog: pytest.LogCaptureFixture
1515+
):
1516+
if not isinstance(storage, SqlRunStorage):
1517+
pytest.skip("storage is not SQL-backed")
1518+
if not self.supports_backfills_count():
1519+
pytest.skip("storage does not support backfill count")
1520+
1521+
origin = self.fake_partition_set_origin("fake_partition_set")
1522+
valid_backfill = PartitionBackfill(
1523+
"valid",
1524+
partition_set_origin=origin,
1525+
status=BulkActionStatus.REQUESTED,
1526+
partition_names=["a", "b", "c"],
1527+
from_failure=False,
1528+
tags={},
1529+
backfill_timestamp=time.time(),
1530+
)
1531+
storage.add_backfill(valid_backfill)
1532+
1533+
with storage.connect() as conn:
1534+
conn.execute(
1535+
BulkActionsTable.insert().values(
1536+
key="corrupt",
1537+
status=BulkActionStatus.REQUESTED.value,
1538+
timestamp=datetime_from_timestamp(time.time()),
1539+
body='{"truncated"',
1540+
)
1541+
)
1542+
1543+
with caplog.at_level("WARNING", logger="dagster"):
1544+
assert storage.get_backfills_count() == 1
1545+
1546+
assert "Skipping backfill corrupt because it could not be deserialized." in caplog.text
1547+
15211548
def test_get_backfills_count_with_tags_skips_corrupt_rows(
15221549
self, storage: RunStorage, caplog: pytest.LogCaptureFixture
15231550
):

0 commit comments

Comments
 (0)