fix APIGW import RestAPI version type mismatch#7787
Conversation
dee68dd to
6d2dea0
Compare
calvernaz
left a comment
There was a problem hiding this comment.
LGTM overall 🚀 . Couple of comments regarding the spec and the field version of the info object.
| # 2. validate the document type, "swagger" or "openapi" | ||
|
|
||
| rest_api.version = resolved_schema.get("info", {}).get("version") | ||
| rest_api.version = str(resolved_schema.get("info", {}).get("version", "")) |
There was a problem hiding this comment.
the info object is required and field title and version the required fields. Translating that to code, I'd prefer not to have a default value that is not valid.
There was a problem hiding this comment.
I see, I didn't dare to change the logic here without having proper testing in place. Should I just leave None instead? Could it create issue down the line with unexpected exception? I suppose I'll just an AWS exception instead?
There was a problem hiding this comment.
Yea absolutely, I don't think it's a big deal. Something to follow up on, there are a couple of libraries to validate swagger 2 vs 3, but we need to create a proper separation.
| }, | ||
| "info": { | ||
| "title": "local-api", | ||
| "version": 1 |
There was a problem hiding this comment.
"version" should be a string by the spec.
There was a problem hiding this comment.
Right, yes. So this was an issue from the sample in the first place. However, AWS accepts this, and it allows us to test that we properly cast it to str, should I leave it this way? Otherwise this fix would not be properly tested.
There was a problem hiding this comment.
Yea, AWS is probably being lenient - the export operation should also reflect that then. 👍
7ccf743 to
9a4b23e
Compare
9a4b23e to
152ce0b
Compare
A user reported an issue in the community slack channel that they could not use a Terraform module creating a RestAPI with API Gateway after 1.4.0.
Terraform was creating then updating the API, we would send a 200 status code but it would keep going (it felt like we were returning something unexpected or wrong).
After running the user sampler and re-using the data sent by Terraform to create a snapshot AWS validated test, it seemed the version type we returned was an
intwhen AWS returns astr. After fixing that, the user sample is running.The snapshot test shows the lack of parity while importing an API, but some of it might be because of how we return the data, and should be fix in our parity effort. Anyway, it's a good indicator on where to focus some effort.