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

REST marshaler prototype using REST Encoder utility #468

Closed

Conversation

skotambkar
Copy link
Contributor

@skotambkar skotambkar commented Jan 7, 2020

Note: Tests fail, due to pending change required to restrict lower casing header's to unmarshaling.

  • Adds prototype for getApiKey operation for apigateway service and putObject operation for s3 service. The prototypes use rest encoder utility from PR aws/aws-sdk-go-v2#449
  • We add Marshal.go for service/apigateway & service/s3. It contains prototyped marshaler operations and functions for respective operations.
  • We also add test to compare the marshaled request using rest encoder utility with the old rest marshaler.

Adds utility for encoding HTTP REST values. Will be used by SDK's generated marshalers.
Adds new implementation of JSON encoder for generated marshaler to use in effort to reduce refection in SDK.
@skotambkar skotambkar changed the base branch from master to feature/GeneratedMarshaling January 7, 2020 16:31
@skotambkar skotambkar self-assigned this Jan 7, 2020
@skotambkar skotambkar force-pushed the proto/restUtil branch 2 times, most recently from ce792b7 to fc72c96 Compare January 7, 2020 16:45
@skotambkar skotambkar changed the title service`: prototype operations for APIGateWay and S3 service and use rest encoder utility service: prototype operations for APIGateWay and S3 service and use rest encoder utility Jan 7, 2020
@skotambkar skotambkar changed the title service: prototype operations for APIGateWay and S3 service and use rest encoder utility service: prototype operations for APIGateWay and S3 service that use rest encoder utility Jan 7, 2020
@skotambkar skotambkar marked this pull request as ready for review January 7, 2020 16:47
@skotambkar skotambkar added the pr/needs-review This PR needs a review from a Member. label Jan 7, 2020
@skotambkar skotambkar requested a review from jasdel January 7, 2020 16:47
@skotambkar skotambkar added the blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. label Jan 7, 2020
service/apigateway/marshal.go Outdated Show resolved Hide resolved
service/apigateway/marshal.go Outdated Show resolved Hide resolved
service/s3/marshal.go Outdated Show resolved Hide resolved
service/s3/marshal.go Show resolved Hide resolved
@skotambkar skotambkar added pr/work-in-progress This PR is a draft and needs further work. and removed pr/needs-review This PR needs a review from a Member. labels Jan 14, 2020
service/s3/marshal.go Outdated Show resolved Hide resolved
@skotambkar skotambkar added pr/ready-to-merge This PR is ready to be merged. and removed pr/work-in-progress This PR is a draft and needs further work. labels Jan 14, 2020
Modifies the REST v2 encoder to not lower-case values, but use the provided Set/Add headers which will canonicalize the headers.
@jasdel
Copy link
Contributor

jasdel commented Jan 20, 2020

Should be able to rebase this PR with the feature branch, pulling in the updated header encoding.

@skotambkar skotambkar removed the blocked Work is blocked on this issue for this codebase. Other labels or comments may indicate why. label Jan 27, 2020
Comment on lines +139 to +144
func marshalPutObjectInputShapeAWSXML(r *aws.Request, input *PutObjectInput) error {
if input.Body != nil {
r.SetReaderBody(input.Body)
}
return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll want to consider how the request body is used across multiple request attempts. This is handled in an awkward way within the SDK's Request type, but leads to the SDK requiring the input.Body to be an io.ReadSeeker. Ideally the SDK should always except an io.Reader, and fail if the request requires a hash to be computed of the body, or disable retries if the body cannot be seeked.

@jasdel jasdel force-pushed the feature/GeneratedMarshaling branch from 875184f to 2550cb3 Compare March 2, 2020 18:24
@skotambkar skotambkar changed the title service: prototype operations for APIGateWay and S3 service that use rest encoder utility REST marshaler prototype using REST Encoder utility Mar 22, 2020
@jasdel
Copy link
Contributor

jasdel commented Aug 5, 2020

smithy branch includes generated rest (de)serializers.

@jasdel jasdel closed this Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-to-merge This PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants