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

Make required context parameters required #2964

Merged
merged 5 commits into from
Aug 31, 2023
Merged

Conversation

rcoh
Copy link
Collaborator

@rcoh rcoh commented Aug 30, 2023

Motivation and Context

When a @contextParam is marked as required, we will enforce it on inputs. Since these fields may influence endpoint, omitting them can result in a different target being hit.

Description

Testing

  • S3 Integration test

Checklist

  • I have updated CHANGELOG.next.toml if I made changes to the smithy-rs codegen or runtime crates
  • I have updated CHANGELOG.next.toml if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates

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

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@rcoh rcoh force-pushed the required-context-params branch from 5d76520 to 38c137b Compare August 30, 2023 16:05
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@rcoh rcoh force-pushed the required-context-params branch from 64840e7 to b403adf Compare August 31, 2023 15:49
@rcoh rcoh marked this pull request as ready for review August 31, 2023 15:50
@rcoh rcoh requested review from a team as code owners August 31, 2023 15:50
@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.

Comment on lines 118 to 123
if let Some(field) = field {
if field.as_ref() != "" {
return Some(field)
}
}
None
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be one line:

Suggested change
if let Some(field) = field {
if field.as_ref() != "" {
return Some(field)
}
}
None
field.map(AsRef::as_ref).filter(|s| s != "")

@rcoh rcoh force-pushed the required-context-params branch from e473125 to de61ad7 Compare August 31, 2023 18:58
@rcoh rcoh enabled auto-merge August 31, 2023 19:08
@rcoh rcoh added this pull request to the merge queue Aug 31, 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.

Merged via the queue into main with commit 2c27834 Aug 31, 2023
@rcoh rcoh deleted the required-context-params branch August 31, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants