Fix header retrieval in REST client #2478
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. DetailsInstructions 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. |
k8s-ci-robot
added
kind/bug
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @franck-ada! |
k8s-ci-robot
added
cncf-cla: no
k8s-ci-robot
added
cncf-cla: yes
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix header retrieval in the REST client to support urllib3 upgrades by adding compatibility checks for the headers attribute. The changes add conditional logic to use the newer .headers attribute when available, falling back to the older .getheaders() and .getheader() methods for backward compatibility.
Key changes:
- Add compatibility checks in
getheaders()method to use.headers.items()when available - Add compatibility checks in
getheader()method to use.headers.get()when available
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kubernetes/client/rest.py
Outdated
| if hasattr('headers', self.urllib3_response): | ||
| return self.urllib3_response.headers.items() | ||
| return self.urllib3_response.getheaders() | ||
|
|
||
| def getheader(self, name, default=None): | ||
| """Returns a given response header.""" | ||
| if hasattr('headers', self.urllib3_response): |
There was a problem hiding this comment.
The arguments to hasattr() are in the wrong order. The correct signature is hasattr(object, name), but this code has hasattr('headers', self.urllib3_response) which checks if the string 'headers' has an attribute named self.urllib3_response.
This should be:
if hasattr(self.urllib3_response, 'headers'):| if hasattr('headers', self.urllib3_response): | |
| return self.urllib3_response.headers.items() | |
| return self.urllib3_response.getheaders() | |
| def getheader(self, name, default=None): | |
| """Returns a given response header.""" | |
| if hasattr('headers', self.urllib3_response): | |
| if hasattr(self.urllib3_response, 'headers'): | |
| return self.urllib3_response.headers.items() | |
| return self.urllib3_response.getheaders() | |
| def getheader(self, name, default=None): | |
| """Returns a given response header.""" | |
| if hasattr(self.urllib3_response, 'headers'): |
kubernetes/client/rest.py
Outdated
| if hasattr('headers', self.urllib3_response): | ||
| return self.urllib3_response.headers.items() | ||
| return self.urllib3_response.getheaders() | ||
|
|
||
| def getheader(self, name, default=None): | ||
| """Returns a given response header.""" | ||
| if hasattr('headers', self.urllib3_response): |
There was a problem hiding this comment.
The arguments to hasattr() are in the wrong order. The correct signature is hasattr(object, name), but this code has hasattr('headers', self.urllib3_response) which checks if the string 'headers' has an attribute named self.urllib3_response.
This should be:
if hasattr(self.urllib3_response, 'headers'):| if hasattr('headers', self.urllib3_response): | |
| return self.urllib3_response.headers.items() | |
| return self.urllib3_response.getheaders() | |
| def getheader(self, name, default=None): | |
| """Returns a given response header.""" | |
| if hasattr('headers', self.urllib3_response): | |
| if hasattr(self.urllib3_response, 'headers'): | |
| return self.urllib3_response.headers.items() | |
| return self.urllib3_response.getheaders() | |
| def getheader(self, name, default=None): | |
| """Returns a given response header.""" | |
| if hasattr(self.urllib3_response, 'headers'): |
kubernetes/client/rest.py
Outdated
|
|
||
| def getheader(self, name, default=None): | ||
| """Returns a given response header.""" | ||
| if hasattr('headers', self.urllib3_response): |
There was a problem hiding this comment.
There is an extra space between if and hasattr. This should be a single space for consistency:
if hasattr(self.urllib3_response, 'headers'):| if hasattr('headers', self.urllib3_response): | |
| if hasattr('headers', self.urllib3_response): |
…#2418) - Replace search() with fullmatch() for strict RFC3339 format validation - Provide clear, actionable error messages with expected format - Add input whitespace stripping before validation - Improve exception handling with specific ValueError messages - Add comprehensive test cases for invalid formats - Addresses reviewer feedback from PR kubernetes-client#2420 Test: Existing tests pass + new test cases added
add support of `.headers` for urlib3 > 2.0.0
k8s-ci-robot
added
size/M
3 participants
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
Special notes for your reviewer:
Does this PR introduce a user-facing change?
NONE