Skip to content

Conversation

@nateprewitt
Copy link
Contributor

@nateprewitt nateprewitt commented Feb 27, 2025

Overview

The PR adds a aws-sdk-signers package with basic SigV4 signing capabilities. This will need to be wired up to our clients in a follow up PR and then added to sign events for the aws-eventstream implementation.

The goal is this package can live inside the smithy-python repo long term while also being able to be distributed independently of any of the smithy-* libraries. There is still some work needed to fully decouple typing for that which will be done in a future PR.

Description of changes:

  • Added SigV4Signer to sign arbitrary requests sychronously.
  • Added AsyncSigV4Signer to sign arbitrary requests asychronously.

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

@nateprewitt nateprewitt requested a review from a team as a code owner February 27, 2025 05:31
@nateprewitt
Copy link
Contributor Author

I'll take a look at the new docformatter tomorrow, some of these suggestions are definitely not what we want. We may need to tweak the config.

JordonPhillips
JordonPhillips previously approved these changes Feb 27, 2025
Copy link
Contributor

@JordonPhillips JordonPhillips left a comment

Choose a reason for hiding this comment

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

Make sure we've got a task to resolve these shared code issues. The IO stuff is easy enough to extract. Having an HTTP package with just protocols might be a bit weird, but probably reasonable?

chain.from_iterable(
fld.as_tuples()
for fld in request.fields.get_by_type(FieldPosition.HEADER)
for fld in request.fields
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this change made?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not actually adding the FieldPosition correctly for headers so we end up with an empty list here. I have a task to go back and work out what's failing. This was a carry over from the original client POC that I still need to sort out.

@nateprewitt nateprewitt merged commit 548b3d0 into develop Feb 28, 2025
1 check passed
@nateprewitt nateprewitt deleted the initial_signers branch February 28, 2025 15:58
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