Skip to content
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

fix: accept empty components property in Open Api Definition #2296

Merged
merged 2 commits into from Jan 18, 2022

Conversation

moelasmar
Copy link
Contributor

Use a default dict in case if a template contains an empty components property in the Open Api definition body instead of failing, as Components property is an optional property in the OpenApi definition.

Issue #, if available:

Description of changes:

Description of how you validated changes:

Checklist:

  • Add/update tests using:
  • make pr passes

Examples?

Please reach out in the comments, if you want to add an example. Examples will be
added to sam init through https://github.com/awslabs/aws-sam-cli-app-templates/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

… property in the Open Api definition body instead of failing, as Components property is an optional property in the OpenApi definition.
Comment on lines 1001 to 1002
components = definition_body.get("components", Py27Dict())
components = components if components else Py27Dict()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't second parameter of get is the default value if the item is None? If so, the line 1002 would be redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, get will only return default value if the components is not the dict at all, but if it is in the dict and its value is None, it will return None and not the default value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, can we add a comment next to it so it won't be removed accidentally in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2022

Codecov Report

Merging #2296 (ed5adfd) into develop (e7a1496) will increase coverage by 0.81%.
The diff coverage is 96.86%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2296      +/-   ##
===========================================
+ Coverage    93.58%   94.39%   +0.81%     
===========================================
  Files           90       97       +7     
  Lines         6124     7087     +963     
  Branches      1260     1434     +174     
===========================================
+ Hits          5731     6690     +959     
+ Misses         183      181       -2     
- Partials       210      216       +6     
Impacted Files Coverage Δ
samtranslator/model/api/http_api_generator.py 91.21% <75.00%> (+0.02%) ⬆️
samtranslator/model/api/api_generator.py 93.70% <89.28%> (-0.66%) ⬇️
samtranslator/model/apigateway.py 96.98% <96.15%> (-0.18%) ⬇️
samtranslator/__init__.py 100.00% <100.00%> (ø)
samtranslator/feature_toggle/dialup.py 100.00% <100.00%> (ø)
samtranslator/feature_toggle/feature_toggle.py 100.00% <100.00%> (+12.16%) ⬆️
samtranslator/intrinsics/actions.py 98.79% <100.00%> (+0.07%) ⬆️
samtranslator/metrics/method_decorator.py 100.00% <100.00%> (ø)
samtranslator/metrics/metrics.py 100.00% <100.00%> (ø)
samtranslator/model/__init__.py 97.64% <100.00%> (ø)
... and 33 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85177ef...ed5adfd. Read the comment docs.

@moelasmar moelasmar merged commit 4b446dd into aws:develop Jan 18, 2022
@moelasmar moelasmar deleted the develop-fix-empty-components branch January 18, 2022 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants