-
Notifications
You must be signed in to change notification settings - Fork 145
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
Do not decode response data in Python2 #225
Conversation
response data is already str in Python2 so it doesn't need to be decoded. Furthermore, as decode() uses ascii by default in PY2, it breaks in case of a non-ascii character the response. Just mimic what kubernetes do in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/rest.py#L216 Fix openshift#224
Hi @wallecan. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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/test-infra repository. |
@wallecan sorry for the delay, was out on PTO! |
response data is already str in Python2 so it doesn't need to be decoded. Furthermore, as decode() uses ascii by default in PY2, it breaks in case of a non-ascii character the response. Just mimic what kubernetes do in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/rest.py#L216 Fix openshift#224 (cherry picked from commit 8e3a2e9)
Is there a workaround for the 0.6.2 version in RHEL ? |
Sort of, but it's a little messy. If you're hitting this I think the only way around it is to make your request with the import six
import json
from kubernetes.config import new_client_from_config
from openshift.dynamic import DynamicClient, ResourceInstance
# These two functions are the up to date versions from openshift/dynamic/client.py
def serialize(resource, response):
try:
return ResourceInstance(resource, load_json(response))
except ValueError:
if six.PY2:
return response.data
return response.data.decode('utf8')
def load_json(response):
if six.PY2:
return json.loads(response.data)
return json.loads(response.data.decode('utf8'))
client = DynamicClient(new_client_from_config())
v1_pods = client.resources.get(api_version='v1', kind='Pod')
# assume the `test` pod in the `test` namespace triggers this bug
my_pod = serialize(v1_pods, v1_pods.get(name='test', namespace='test', serialize=False)) |
Thanks for the help ! I will try that Edit: seems to be working |
response data is already str in Python2 so it doesn't need to be decoded. Furthermore, as decode() uses ascii by default in PY2, it breaks in case of a non-ascii character the response. Just mimic what kubernetes do in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/rest.py#L216 Fix openshift#224
/cherrypick release-0.6 |
@fabianvf: new pull request created: #329 In response to this:
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/test-infra repository. |
response data is already str in Python2 so it doesn't need to be decoded. Furthermore, as decode() uses ascii by default in PY2, it breaks in case of a non-ascii character the response. Just mimic what kubernetes do in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/rest.py#L216 Fix openshift#224
response data is already str in Python2 so it doesn't need to be decoded.
Furthermore, as decode() uses ascii by default in PY2, it breaks in case of a non-ascii character the response.
Just mimic what kubernetes do in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/rest.py#L216
Fix #224