fix validator and model check during invocation #7846
Conversation
|
Changes Unknown when pulling d290bbf on fix/7842 into ** on master**. |
ea9006d to
e1f6be5
Compare
| try: | ||
| model = self.apigateway_client.get_model( | ||
| restApiId=self.context.api_id, | ||
| modelName=schema_name, | ||
| ) | ||
| except ClientError as e: | ||
| # This should not happen, as we validate that we cannot delete a Model if it's used |
There was a problem hiding this comment.
I'm always a bit wary of "should not happen" comments - should we simply remove the try/except handling in this case, if we think it is not a valid code path?
There was a problem hiding this comment.
Yes, I think it can happen for now until #7872 is merged. I'd be for letting it be there, I'll mark it with a TODO instead, until we're sure this code path can't happen anymore.
And maybe somehow the API could be deleted at the same time, triggering an exception. I'll rewrite the comment instead, this might happen in weird case and it's better to log the error and refuse the request than getting some 500 error.
| @@ -513,6 +513,11 @@ def update_method( | |||
| continue | |||
|
|
|||
| elif path == "/requestValidatorId" and value not in rest_api.validators: | |||
| if not value: | |||
| # you can remove a requestValidator by passing an empty string as a value | |||
There was a problem hiding this comment.
A wonderful demonstration of the weirdness of API GW patch operations! 😄
| requestParameters={"method.request.path.test": True}, | ||
| ) | ||
|
|
||
| apigateway_client.put_integration( |
There was a problem hiding this comment.
nit: for simple integrations, we could try leveraging and extending the create_rest_api_with_integration fixture. It currently only supports dynamodb and kinesis integrations, but Lambda would be a great addition.. 👍
There was a problem hiding this comment.
Makes sense! I need to take a look at it, and how we can customise it. I guess we can always create simple integrations and use update_* afterwards, for example the requestParameters in that case. There are so many parameters possible.
I'll take a good look a those once I'm starting with integrations. This PR was a bit outside my "normal" work for now, as I was fixing the change in logic now that we are raising exceptions for non-existing resources. 😄
We had a wrong assumption when using a
RequestValidatorand validating the body of a request. If a method did not have arequestModelset, we would reject the request. However, AWS uses the defaultEmptymodel. We now follow the same logic.We also did not have validation for validators before, and would return an empty response if no validators existed with that ID. Now, in parity with AWS, it raises an exception instead of returning
None.However, that case should not happen because it's not possible to delete an existing
RequestValidatorif it's used by aMethodin AWS. I'll try to implement this behaviour in following PR.We needed to change the code in
invocations.pyto reflect the change when checking if a request is valid.note: created a
test_apigateway_common, where we could test "integrations", or how different RestAPI resources work together, on a level above the CRUD one. If you have a better name than "common", I'm all ears 😄fixes in part #7842