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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CloudFormation Registry resource schema support (--registry-schemas) #1732
CloudFormation Registry resource schema support (--registry-schemas) #1732
Changes from all commits
a706916
bef33b5
a4bae45
7e84e41
d980723
44bc4c0
2d5a274
60f0a9e
8cb475a
334b54a
6c0b32b
714d807
f7d7ade
5c82758
6a3951a
1f6db95
e0a039d
0258254
3281ba6
90dc2ae
758dd5f
8504bf1
3fea969
82a5381
2d65480
64e4b96
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outside packages shouldn't be using run_cli by any concerns with the ordering here? I've been adding parameters and setting a default value to keep some backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only found this from searching: https://github.com/Cloudzero/samwise/blob/793463e0774f9d1c702f4252377f19fa3408a558/samwise/__main__.py#L129 which looks already broken from #1411
cc @silvexis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have generally been debating compatibility for reliance on undocumented internal functions/variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the big problems we are going to have with using this package is handling
Fn::If
or other intrinsic functions. Either way we are going to have to handle it outside of the schema validation which we have logic to handle or we are going to have to rewrite these validation functions that this package provides to handle these things differently.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this says ignore intrinsic function errors but doesn't handle more complex Fn::If scenarios.
We basically don't get a failure even though there could be one in the provided scenario.