Report an oversized expects= integer instead of panicking - #1002
Open
arpitjain099 wants to merge 1 commit into
Open
Report an oversized expects= integer instead of panicking#1002arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
checkInt tries Int64() then Uint64() and panics when neither fits. The value comes straight from an overlay annotation, so a template with #@overlay/match by=overlay.all, expects=99999999999999999999999999999999999999999 crashed ytt with 'Unsure how to convert starlark.Int to int' rather than telling the author what was wrong. A negative bignum does the same. Such a value can never equal a node count, so report it like the other malformed expects= inputs already are. Signed-off-by: Arpit Jain <arpitjain099@gmail.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
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.
MatchAnnotationExpectsKwarg.checkInttriesInt64()and thenUint64(), and panics if neither conversion succeeds:That value comes straight out of the template, so an overlay like this crashes ytt:
#@overlay/match by=overlay.all, expects=99999999999999999999999999999999999999999A negative bignum does the same. An integer that fits in neither type can never equal a node count, so it is a bad input rather than an impossible state, and the other malformed
expects=values are already reported as errors. This returns one in the same shape.The test drives it through
RunWithFiles, which is the path the CLI takes. It panics without the change.Noted while verifying, not fixed here since it is a separate code path:
--data-value-yaml n=<bignum>panics inpkg/template/core/go_value.gowith a different message, so that one wants its own change.