-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix: correect header retrieval for urllib3 compatibility #2479
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
base: master
Are you sure you want to change the base?
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: franck-ada The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes header retrieval in the RESTResponse class to support urllib3 v2.x compatibility. The PR addresses issue #2280 by adding compatibility checks that prefer the newer headers attribute (available in urllib3 v2.x) before falling back to the deprecated getheaders() and getheader() methods.
Note: The PR title contains a typo: "correect" should be "correct".
Key changes:
- Added
hasattr()checks to detect the presence of theheadersattribute on urllib3 response objects - Updated
getheaders()to useheaders.items()when available, falling back togetheaders()for older versions - Updated
getheader()to useheaders.get()when available, falling back togetheader()for older versions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.data = resp.data | ||
|
|
||
| def getheaders(self): | ||
| """Returns a dictionary of the response headers.""" |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring states "Returns a dictionary of the response headers" but the method returns headers.items() which returns an iterable of tuples (key-value pairs), not a dictionary. Consider updating the docstring to accurately reflect the return type, such as "Returns an iterable of header key-value pairs" or "Returns header items as tuples".
| """Returns a dictionary of the response headers.""" | |
| """Returns an iterable of header key-value pairs (tuples).""" |
- add support for headers
a602e3f to
7a59580
Compare
|
Here is another occurrence of python/kubernetes/client/exceptions.py Line 91 in a49d85d
From my sniff testing, this needs to be changed as well. |
I was looking at it. this |
@franck-ada I was testing an ansible playbook with urllib3 2.6.0 patched like this: When trying to delete a configmap that doesn't exist through ansible's I had to add this additional patch to get past this error: Unfortunately, I didn't find a way yet to have ansible print the full stack trace. |
What type of PR is this?
/kind bug
/kind deprecation
What this PR does / why we need it:
Correct issue link to upgrade of urlib3
Which issue(s) this PR fixes:
Fixes #2280 / #2477
Special notes for your reviewer:
Does this PR introduce a user-facing change?
NONE