Conversation
…efaults This reverts commit b74fff0 (MaterializeInc#38908). MaterializeInc#38918 landed fifteen minutes before MaterializeInc#38908 and fixed the same nightly failure by no longer downloading the published MinIO manifest at all: sample-minio.yaml is always copied from the in-tree file. MaterializeInc#38908 was written against the earlier tree and merged without a rebase, so its branch on `file == "sample-minio.yaml"` can never match, and the regex and the import-time `MINIO_IMAGE` read are dead code. Two comments in the same loop also disagreed about whether released-version iterations exercise the published manifest. Removing the dead hunks leaves MaterializeInc#38918's approach as the fix and drops an import-time file read whose only failure mode would have been breaking pipeline generation for nothing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
#38918 and #38908 both fixed the
Orchestratord + documentation defaultsnightly failure caused by MinIO deleting its Docker Hub repository, and they took opposite approaches. #38918 stopped downloading the publishedsample-minio.yamland always applies the in-tree manifest. #38908 kept downloading it and rewrote itsimage:line to the image the current tree uses.#38918 merged first (
0d4867fce5, 19:01 UTC). #38908 was written against the earlier tree and merged fifteen minutes later without a rebase (b74fff0dd7), so both are onmainand only #38918's has any effect:filesno longer containssample-minio.yaml, so theif file == "sample-minio.yaml"branch can never match, and the regex and the import-timeMINIO_IMAGEread are dead code. Two comments in the same loop body also give opposite answers about whether released-version iterations exercise the published manifest.Description
Reverts
b74fff0dd7(#38908), leaving #38918's approach as the fix. This removes the dead branch, theMINIO_IMAGEhelper and its import-time read ofmisc/helm-charts/testing/minio.yaml, and thereimport that only the dead branch used. The import-time read is worth removing on its own:test/orchestratord/mzcompose.pyis imported during pipeline generation, so a future change to the manifest's shape would have failed the whole composition for no benefit.If exercising the published manifest with only its image repointed turns out to be the behavior we want, that is a deliberate reversal of #38918 to make with its author, not something to leave half-present on
main.Verification
Existing test; both shards of
Orchestratord + documentation defaultsin nightly run it, and the workflow's behavior is unchanged from what #38918 established. Loading the composition fromtest/orchestratordwith../../bin/mzcompose list-workflowsstill succeeds.Surfaced by the post-merge QA review on #38908.
🤖 Generated with Claude Code