-
Notifications
You must be signed in to change notification settings - Fork 592
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
--update-specs even if official CloudFormation Resource Specification hasn't changed #1554
Conversation
935ec67
to
2deb44a
Compare
This comment has been minimized.
This comment has been minimized.
# commented out for now due to https://github.com/aws-cloudformation/cfn-python-lint/pull/1383#issuecomment-629891506 | ||
# if not url_has_newer_version(url): | ||
# return |
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.
It would be better to keep the url_has_newer_version()
call, but instead of returning from the function continue on instead, just skipping the downloads. This would let you do the patching regardless of whether new external specs are available or not.
# Check to see if we already have the latest version, otherwise download it
if url_has_newer_version(url):
spec_content = get_url_content(url, caching=True)
spec = json.loads(spec_content)
else:
with open(filename, 'r') as f:
spec = json.load(f)
Tests will likely need to be adjusted.
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.
It would be better to keep the
url_has_newer_version()
call, but instead of returning from the function continue on instead, just skipping the downloads. This would let you do the patching regardless of whether new external specs are available or not.
@Tro95 Agreed, just using this in the meantime. I'm not sure we've stored the previous unpatched official CloudFormation Resource Specifications anywhere where we could easily do this. Think we only have the final merged results, so it might take more time to properly fix
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.
@Tro95 @kddejong @miparnisari
maybe just an undocumented --force
flag for now that skips this url_has_newer_version()
check?
cfn-lint --update-specs --force
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.
That seems like a good short-term solution
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.
Went ahead and created a force option. Please take a look #2334 |
#1383 (comment)
cfn-lint --update-specs
patchesExtendedSpecs
andbotocore
data intoCloudSpecs
in addition to newly downloaded official CloudFormation Resource SpecificationsExtendedSpecs/$REGION/05_pricing_property_values.json
written byscripts/update_specs_from_pricing.py
ExtendedSpecs/$REGION/06_ssm_service_removal.json
written byscripts/update_specs_services_from_ssm.py
ExtendedSpecs/$REGION/07_ssm_service_addition.json
written byscripts/update_specs_services_from_ssm.py
the rest of
ExtendedSpecs
is hand maintained