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

Small tweaks to the HTTP wrapper #3099

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Small tweaks to the HTTP wrapper #3099

merged 1 commit into from
Oct 30, 2023

Conversation

rcoh
Copy link
Collaborator

@rcoh rcoh commented Oct 25, 2023

Motivation and Context

  • small improvements / consistency updates to http wrapper
  • fix bug where .insert was called but it should have been append

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rcoh rcoh requested a review from a team as a code owner October 25, 2023 18:20
@rcoh rcoh mentioned this pull request Oct 25, 2023
9 tasks
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@github-actions
Copy link

@rcoh rcoh added the breaking-change This will require a breaking change label Oct 25, 2023
@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@@ -132,7 +132,7 @@ fn apply_endpoint(
for (header_name, header_values) in endpoint.headers() {
request.headers_mut().remove(header_name);
for value in header_values {
request.headers_mut().insert(
request.headers_mut().append(
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to have a unit test for the use of append if we don't have one.

@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@@ -404,8 +404,10 @@ impl Headers {
/// Removes all headers with a given key
///
/// If there are multiple entries for this key, the first entry is returned
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be more explicit and add something like "all other entries are discarded" or "to get all values for a given key, keep calling this until it returns None"?

@github-actions
Copy link

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@rcoh rcoh added this pull request to the merge queue Oct 30, 2023
Merged via the queue into main with commit 5a8b07c Oct 30, 2023
40 of 41 checks passed
@rcoh rcoh deleted the http-fixes branch October 30, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will require a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants