Skip to content

Conversation

@alextwoods
Copy link
Contributor

Description of changes:
We were previously not setting the host header which was causing requests to fail. The underlying CRT client does not set it. The aihttp client does set this automatically so our adapter didn't need to set it explicitly.

TODO: Needs added tests.

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

@alextwoods alextwoods requested a review from a team as a code owner March 11, 2025 21:00
@alextwoods alextwoods changed the title Add host Add host header when unset to crt requests Mar 11, 2025
@nateprewitt
Copy link
Contributor

😬 wonderful, I think every Python client does this for you so the CRT is an outlier here. I think this is fine conceptually but I know some libraries don't play nicely with the end-user setting this. Did we validate aiohttp still works with this change?

nateprewitt
nateprewitt previously approved these changes Mar 11, 2025
Copy link
Contributor

@jonathan343 jonathan343 left a comment

Choose a reason for hiding this comment

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

LGTM

@alextwoods alextwoods merged commit 4c425c2 into smithy-lang:develop Mar 11, 2025
2 checks passed
@alextwoods alextwoods deleted the add_host branch March 11, 2025 23:31
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.

3 participants