Skip to content

urllib3: Fix headers when used in kwargs#4143

Closed
kjprice wants to merge 4 commits intoopen-telemetry:mainfrom
kjprice:urllib3-fix-headers-kwargs
Closed

urllib3: Fix headers when used in kwargs#4143
kjprice wants to merge 4 commits intoopen-telemetry:mainfrom
kjprice:urllib3-fix-headers-kwargs

Conversation

@kjprice
Copy link
Copy Markdown

@kjprice kjprice commented Jan 26, 2026

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #4115

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please see this discussion on how to test.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jan 26, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@kjprice
Copy link
Copy Markdown
Author

kjprice commented Jan 26, 2026

CLA Not Signed

I have signed the agreement now.

return method


def _normalize_headers_to_kwargs(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd prefer that we use a more robust method to accomplish this. I'd suggest we use a combination of inspect.signature and signature.bind to bind the arguments.

@herin049
Copy link
Copy Markdown
Contributor

We also need to add tests for this as well. You mentioned you don't have a large amount of time to contribute so I've implement the changes using inspect.signature with signature.bind and additional tests here: #4144

@kjprice
Copy link
Copy Markdown
Author

kjprice commented Jan 26, 2026

Yay! Thanks @herin049!

@rite7sh
Copy link
Copy Markdown
Contributor

rite7sh commented Jan 26, 2026

hey, it might be worth adding a regression test that explicitly passes headers positionally with instrumentation enabled, just to lock this behavior in and avoid regressions? Otherwise this approach looks solid to me

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.

TypeError: HTTPConnectionPool.urlopen() got multiple values for argument 'headers'

4 participants