Fail to build for services with unsupported payloads #1187
Conversation
|
To note, this is what we could do post Pinpoint Email being updated to not use GET/DELETE with body. I guess I can make the code generator just log a warning until Pinpoint Email is changed. |
| */ | ||
| private void validateOperations(IntermediateModel model) { | ||
| List<String> methods = model.getOperations() | ||
| .entrySet() |
There was a problem hiding this comment.
values().stream()? You're not using the keys below.
| }, | ||
| "documentation":"<p>Used to associate a configuration set with a dedicated IP pool.</p>" | ||
| }, | ||
| "Destination":{ |
There was a problem hiding this comment.
Do we need all this, or can it be shortened?
| if (NO_BODY_METHODS.contains(request.method()) && request.contentStreamProvider().isPresent()) { | ||
| SdkStandardLogger.REQUEST_LOGGER.warn(() -> NO_BODY_METHODS + | ||
| " requests with bodies are not supported by the default SDK clients."); | ||
| } |
There was a problem hiding this comment.
Is this needed, if we have generator validation?
There was a problem hiding this comment.
I added this in case someone was using our core with their own plugged in client that supported GET requests with a body. Very unlikely to occur and won't occur for any AWS service. Can just throw or remove here if preferred though.
There was a problem hiding this comment.
I'd favor throwing or removing. Feels like complexity (and work the SDKs have to do with every method call) for not a lot of gain.
spfink
force-pushed
the
finks/unsupported-payloads
branch
from
9ddf19c to
f493979
Compare
Codecov Report
@@ Coverage Diff @@
## master #1187 +/- ##
============================================
+ Coverage 73.73% 73.76% +0.03%
Complexity 660 660
============================================
Files 844 844
Lines 25936 25948 +12
Branches 1986 1988 +2
============================================
+ Hits 19123 19140 +17
+ Misses 5954 5949 -5
Partials 859 859
Continue to review full report at Codecov.
|
|
Is there a way to know which APIs are impacted by this? I have a Lambda API call to GetLayerVersionByArn that's failing, and this seems possibly related. I think it's similar to 1078, but it's unclear. I'll note that v1 of the SDK works without issue, and the same API call in Python has no problems. |
Revert "Merge pull request #1187 from aws/finks/unsupported-payloads"
…e9d63e067 Pull request: release <- staging/63d2f8e1-88bd-4766-a368-60fe9d63e067
With this change the code generator will no longer allow models to be build that have bodies for GET, DELETE or HEAD requests.
If a body is detected with one of these methods, the core will log a warning. I can switch this to also throwing an error but figured to throw a warning in case someone wanted to use our core and plugabble HTTP to support these.
Testing
Added test for code generation failure.
Types of changes
Checklist
mvn installsucceedsLicense