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

Add Request Model Support #948

Merged
merged 16 commits into from
Jun 13, 2019
Merged

Add Request Model Support #948

merged 16 commits into from
Jun 13, 2019

Conversation

beck3905
Copy link
Contributor

@beck3905 beck3905 commented Jun 3, 2019

Issue #, if available:
#895 and #931

Description of changes:
Added support for listing Models in AWS::Serverless:API and defining a model to be used in the Api event source.

Description of how you validated changes:
unit tests
end to end tests
ran transform locally and ran created a stack from the transformed template

Checklist:

  • Write/update tests
  • make pr passes
  • Update documentation

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

@codecov-io
Copy link

codecov-io commented Jun 4, 2019

Codecov Report

Merging #948 into develop will decrease coverage by 0.11%.
The diff coverage is 90%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #948      +/-   ##
===========================================
- Coverage    94.78%   94.67%   -0.12%     
===========================================
  Files           69       69              
  Lines         3067     3135      +68     
  Branches       578      601      +23     
===========================================
+ Hits          2907     2968      +61     
- Misses          84       85       +1     
- Partials        76       82       +6
Impacted Files Coverage Δ
samtranslator/model/sam_resources.py 95.5% <ø> (ø) ⬆️
samtranslator/model/api/api_generator.py 93.35% <83.33%> (-1.38%) ⬇️
samtranslator/model/eventsources/push.py 88.85% <88.88%> (ø) ⬆️
samtranslator/swagger/swagger.py 97.61% <96.77%> (-0.12%) ⬇️

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 c6dc591...10d063f. Read the comment docs.

@beck3905
Copy link
Contributor Author

beck3905 commented Jun 5, 2019

I'm not really sure what needs to be done to update documentation. I couldn't see in the repo how the documentation gets generated in order to update it.

@@ -37,6 +37,7 @@ def __init__(self, doc):
self.paths = self._doc["paths"]
self.security_definitions = self._doc.get("securityDefinitions", {})
self.gateway_responses = self._doc.get(self._X_APIGW_GATEWAY_RESPONSES, {})
self.definitions = self._doc.get('definitions', {})
Copy link
Contributor

Choose a reason for hiding this comment

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

To make this work with Openapi 3, you would need to move definitions under $/components/schemas.
You could add it as a post process similar to the auth components here - https://github.com/awslabs/serverless-application-model/blob/develop/samtranslator/model/api/api_generator.py#L293

Here is a link on whats changed from swagger 2.0 to openapi 3.0 -
https://blog.readme.io/an-example-filled-guide-to-swagger-3-2/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@praneetap I've updated the PR to include openapi support. Will you please have a look?

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes look good to me, have you tried to deploy an openapi transformed template manually on cloud formation?
We would also really appreciate it if you add an example to the examples/.

Copy link
Contributor

@keetonian keetonian left a comment

Choose a reason for hiding this comment

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

The documentation is manually generated; if you could add the new Models field here that would be great!

@beck3905
Copy link
Contributor Author

@praneetap @keetonian I've added an example and update documentation as requested.

@keetonian keetonian merged commit 1bb186c into aws:develop Jun 13, 2019
@beck3905
Copy link
Contributor Author

@keetonian Thank you for the approval and merge. What is the process once merged for new features to be released and generally available?

@keetonian
Copy link
Contributor

keetonian commented Jun 17, 2019

@beck3905 we periodically pull in changes to the internal version of the transform, run a suite of integration tests, and gradually release the change through supported AWS regions. We post on corresponding GitHub issues once a change has been deployed in all regions and post release notes to this repo.

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