bugfix(dagger): align ktx2/e2e CLI names with engine kebab-case (#240)#256
Merged
bugfix(dagger): align ktx2/e2e CLI names with engine kebab-case (#240)#256
Conversation
dagger's Go-side CLI converts the camelCase function name from the Python SDK to kebab-case, splitting on every letter↔digit and lower→upper boundary. So `derive_ktx2` → `deriveKtx2` → `derive-ktx-2` (not `derive-ktx2`), and `test_e2e` → `testE2E` → `test-e-2-e` (not `test-e2e`). The derive.yml ternary and the e2e.yml step were both invoking the un-split form and failing with `unknown command "derive-ktx2"` / `"test-e2e"`. Option B from the issue: keep the Python @function names as-is and update the workflows + header comments to match what dagger actually exposes. A new contract test (tests/test_dagger_function_names.py) parses every @function in main.py, derives the kebab CLI name with the same rule the engine uses, and asserts both workflows reference the canonical names. Future @function renames that drift from the workflow break in unit tests instead of CI dispatches.
Contributor
Author
Smoke dispatchTriggered: https://github.com/MorePET/mat-vis/actions/runs/25175351995 Load-bearing verification already in: the End-to-end smoke (full transcode of all 3 sources at 1k → ktx2-1k) is still running; will update with conclusion. |
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
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.
Summary
Fixes #240.
dagger call derive-ktx2was failing in.github/workflows/derive.ymlwithError: unknown command "derive-ktx2". Root cause: dagger's Go-side CLI splits letter↔digit boundaries when converting camelCase to kebab, so the Python@function async def derive_ktx2is exposed asderive-ktx-2(with split digit), notderive-ktx2.I confirmed empirically by running
dagger functions -m .daggeron engine v0.20.6:The same drift was hiding in
e2e.yml(test-e2e→ actuallytest-e-2-e) — fixed in the same commit since it's the identical root cause.Option chosen
Option B from the issue: keep the Python
@functionnames as-is, update the workflow CLI invocations + header comments to match what dagger actually exposes. Rationale:derive_ktx2→derive_ktx_2(Option A) is uglier in Python with no upside.@function(name=...)override is passed straight through to the engine's GraphQL surface, then re-kebab-converted by the same Go rule — so it can't producederive-ktx2either. Option C isn't actually available without engine-level support.Changes
.github/workflows/derive.yml— ternary now emits'derive-ktx-2'; header comment + new#240note documents the conversion rule..github/workflows/e2e.yml— step now invokestest-e-2-e; same#240doc note..dagger/src/mat_vis_ci/main.py— module docstring updated to the canonical kebab names; added a docstring note onderive_ktx2explaining the engine's CLI surface.tests/test_dagger_function_names.py(new) — contract test that parses every@functionfrommain.py, derives the kebab CLI name with the same rule the engine uses, and asserts both workflows reference the canonical names. Future renames that desync from the workflow trip a unit test instead of a CI dispatch.The kebab heuristic in the test was cross-checked against the live
dagger functionsoutput for all 22 currently-decorated@functionmethods — every prediction matches.Test plan
uv run pytest tests/test_dagger_function_names.py tests/test_dagger_bake_argv.py(17 passed)python3 scripts/check-dagger-shell-safety.py(passed)ruff check tests/test_dagger_function_names.py(passed)gerchowl/mat-vis-tst@v0.0.0-phase7-smoke(next step — will paste run URL into this PR once it lands)