Skip to content

[ci] test delta restart against MinIO in CI#6131

Merged
swanandx merged 1 commit into
mainfrom
fix-output-restart-test
Apr 26, 2026
Merged

[ci] test delta restart against MinIO in CI#6131
swanandx merged 1 commit into
mainfrom
fix-output-restart-test

Conversation

@swanandx
Copy link
Copy Markdown
Member

@swanandx swanandx commented Apr 25, 2026

test_delta_output_restart.py failed in k8s because the pipeline pod and the pytest runner do not share /tmp.

check commit message for details

Describe Manual Test Plan

ran locally, didn't test against MINIO

Comment thread python/tests/platform/test_delta_output_restart.py Outdated
Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid CI-fixup refactor. The hand-rolled delta-log replay is replaced with DeltaTable(...).to_pyarrow_dataset().count_rows(), which reads the same active-file set the old code computed manually — so the row-count assertion semantics are preserved across both backends.

Two nits inline; not blockers.

Comment thread python/tests/platform/test_delta_output_restart.py Outdated
Comment thread python/tests/platform/test_delta_output_restart.py Outdated
@swanandx swanandx force-pushed the fix-output-restart-test branch from ee71ce6 to 4fa6e1c Compare April 25, 2026 18:47
@swanandx swanandx requested review from gz and mythical-fred April 25, 2026 18:48
Copy link
Copy Markdown

@mythical-fred mythical-fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice rework — the DeltaTestLocation dataclass cleanly factors out the dual-backend storage logic, and log_json_paths() / read_text() make it easy for other tests in this dir to do delta-log inspection against either backend.

One thing from my previous review wasn't actually picked up: the schema-column check in test_modified_view_re_truncates_on_resume is still missing. The test still asserts only that the row count drops to 0 and that 30 new rows arrive — both of which would also pass if the connector simply re-truncated and re-emitted with the old schema. Now that log_json_paths() / read_text() exist, restoring it is ~5 lines. I'm not going to block on it; LGTM.

Copy link
Copy Markdown
Contributor

@gz gz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't argue with fred saying "Nice rework"

@swanandx swanandx added this pull request to the merge queue Apr 26, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 26, 2026
@swanandx swanandx added this pull request to the merge queue Apr 26, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 26, 2026
Rework test_delta_output_restart.py to pick its storage backend at
runtime via DeltaTestLocation:

* Local runs use a `file://` URI under /tmp.
* CI runs (CI=true) use the in-cluster MinIO endpoint, so the
pipeline pod and the test runner reach the table over S3.

Move shared CI helpers (DeltaTestLocation, MinIO/Kafka endpoints,
env helpers) into python/tests/utils.py. They are imported only by
the delta test, after `pytest.importorskip("deltalake")`, so jobs
that don't run delta (runtime/workload) never load deltalake.

Row counts come from `DeltaTable.count()`, which reads the per-file
numRecords stats already in the delta log

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
@swanandx swanandx force-pushed the fix-output-restart-test branch from 8a21006 to e14f9ef Compare April 26, 2026 17:08
@swanandx swanandx enabled auto-merge April 26, 2026 17:08
@swanandx swanandx added this pull request to the merge queue Apr 26, 2026
Merged via the queue into main with commit 724f28f Apr 26, 2026
1 check passed
@swanandx swanandx deleted the fix-output-restart-test branch April 26, 2026 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants