feat(protocol-http): implement SRA HttpResponse #4520
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kuhe
reviewed
Mar 13, 2023
|
|
||
| type HttpResponseOptions = Partial<HttpMessage> & { | ||
| statusCode: number; | ||
| }; | ||
|
|
||
| export interface HttpResponse extends IHttpResponse {} | ||
| type HttpResponseFromOptions = { |
There was a problem hiding this comment.
this looks like it's part of the public API, so it should be exported, along with HttpResponseOptions
There was a problem hiding this comment.
yea I wasn't sure why HttpResponse didn't export the options, but HttpRequest did. I'll update it
kuhe
approved these changes
Mar 13, 2023
This change implements the core functionality of HttpResponse for SRA. New properties were added to the HttpResponse class, old properties and interfaces were deprecated. To maintain compatibility with old implementation, Proxy was added for headers property, and Object.defineProperties is used with getters/setters for other old properties. This change does not include updates to the SDK to use new functionality. Additional notes: * HttpResponse.isInstance was left unchanged to maintain compatibility. Previous implementation only checked for specific properties, so it would return true when passed the HttpResponse interface from types package. * smithy-lang/smithy-typescript#698 is a related change that ensures protocol tests are using HttpResponse constructor. Due to type changes in HttpResponse, this is required.
milesziemer
force-pushed
the
update-httpresponse
branch
from
36f4ad9 to
15d574f
Compare
AndrewFossAWS
approved these changes
Mar 14, 2023
|
Closed while issues with header proxy are worked out. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This change implements the core functionality of HttpResponse for SRA. New properties were added to the HttpResponse class, old properties and interfaces were deprecated. To maintain compatibility with old implementation, Proxy was added for headers property, and Object.defineProperties is used with getters/setters for other old properties. This change does not include updates to the SDK to use new functionality.
Additional notes:
Testing
CI,
yarn test:integration:legacyBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.