fix(template): handle empty or non-mapping products.yml cleanly#199
Open
plusky wants to merge 1 commit into
Open
fix(template): handle empty or non-mapping products.yml cleanly#199plusky wants to merge 1 commit into
plusky wants to merge 1 commit into
Conversation
load_template returned YAML(typ='safe').load(f) verbatim: an empty or
comment-only products.yml loads as None and a top-level sequence loads
as a list. Both crash consumers far from the parse site with
AttributeError on .keys()/.items():
- the REPA shell-completion callback _complete_repa promises that any
config failure collapses to an empty completion list, but it only
caught OSError/YAMLError, so the AttributeError escaped into the
user's shell mid-keystroke;
- KnownProducts._srun aborted 'repose known-products' with a raw
traceback instead of a clean empty listing.
Fix at the source: load_template now normalizes None to {} so an
empty or comment-only template means 'no products' everywhere, and
raises TemplateError (a ValueError subclass) naming the offending
type for any other non-mapping top level. _dispatch translates
TemplateError into the same one-line error + exit 2 that the other
config-load failures (missing file, permission denied, YAMLError)
already get, so every command fails cleanly instead of with a raw
traceback; subclassing keeps unrelated ValueErrors raised during
command execution (unresolvable REPAs, host repository read
failures) out of that handler. _complete_repa additionally swallows
ValueError so a malformed template still collapses to empty
completions. known-products with an empty template now lists nothing
and exits 0.
Regression tests, all failing on the pre-fix code: the loader and
the completion callback each cover the empty, comment-only and
top-level-sequence templates; known-products covers empty and
comment-only (clean empty listing, exit 0) plus the sequence case
(TemplateError propagates out of run() for _dispatch to translate);
a CLI-level test drives known-products end to end with a list-shaped
products.yml and asserts the one-line error + exit 2 with no
traceback.
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.
What
load_template returned YAML(typ='safe').load(f) verbatim: an empty or
comment-only products.yml loads as None and a top-level sequence loads
as a list. Both crash consumers far from the parse site with
AttributeError on .keys()/.items():
config failure collapses to an empty completion list, but it only
caught OSError/YAMLError, so the AttributeError escaped into the
user's shell mid-keystroke;
traceback instead of a clean empty listing.
Fix at the source: load_template now normalizes None to {} so an
empty or comment-only template means 'no products' everywhere, and
raises TemplateError (a ValueError subclass) naming the offending
type for any other non-mapping top level. _dispatch translates
TemplateError into the same one-line error + exit 2 that the other
config-load failures (missing file, permission denied, YAMLError)
already get, so every command fails cleanly instead of with a raw
traceback; subclassing keeps unrelated ValueErrors raised during
command execution (unresolvable REPAs, host repository read
failures) out of that handler. _complete_repa additionally swallows
ValueError so a malformed template still collapses to empty
completions. known-products with an empty template now lists nothing
and exits 0.
Regression tests, all failing on the pre-fix code: the loader and
the completion callback each cover the empty, comment-only and
top-level-sequence templates; known-products covers empty and
comment-only (clean empty listing, exit 0) plus the sequence case
(TemplateError propagates out of run() for _dispatch to translate);
a CLI-level test drives known-products end to end with a list-shaped
products.yml and asserts the one-line error + exit 2 with no
traceback.
Note: this branch and the
fix(cli)no-color branch from this batch both touchrepose/cli.pyand will conflict; either order merges with a trivial rebase, which I'll provide.Verification
Full gate green (ruff format/check, ty, pytest: 557 passed, coverage 82.53%). Tests at three layers: loader (empty / comment-only / sequence), completion (collapses to empty), known-products (empty / comment-only / sequence), plus a CLI end-to-end test asserting a list-shaped products.yml gets the clean one-liner and exit 2 instead of a traceback. The non-mapping error is a dedicated TemplateError (ValueError subclass) so _dispatch does not mislabel unrelated ValueErrors. Verified to fail pre-fix. Reviewed by a fan-out adversarial code review (two finder lenses per branch, two verifier lenses per finding); all confirmed findings were addressed before opening.
AI-assisted.