Skip to content
This repository has been archived by the owner on Mar 13, 2022. It is now read-only.

Fix data decode for watch/watch.py:49 #138

Closed
wants to merge 1 commit into from
Closed

Fix data decode for watch/watch.py:49 #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions watch/watch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import json
import pydoc
import six

from kubernetes import client

Expand Down Expand Up @@ -45,19 +46,26 @@ def _find_return_type(func):

def iter_resp_lines(resp):
prev = ""
newline_symbol = "\n"
if six.PY3:
prev = b""
newline_symbol = b"\n"
for seg in resp.read_chunked(decode_content=False):
if isinstance(seg, bytes):
seg = seg.decode('utf8')
seg = prev + seg
lines = seg.split("\n")
if not seg.endswith("\n"):
lines = seg.split(newline_symbol)
if not seg.endswith(newline_symbol):
prev = lines[-1]
lines = lines[:-1]
else:
prev = ""
if six.PY3:
prev = b""
for line in lines:
if line:
yield line
if isinstance(line, bytes):
yield line.decode('utf8', 'replace')
else:
yield line


class Watch(object):
Expand Down
18 changes: 18 additions & 0 deletions watch/watch_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import six
import unittest

from mock import Mock, call
Expand All @@ -40,6 +41,17 @@ def test_watch_with_decode(self):
'{"type": "ADDED", "object": {"metadata": {"name": "test3",'
'"resourceVersion": "3"}, "spec": {}, "status": {}}}\n',
'should_not_happened\n'])
if six.PY3:
fake_resp.read_chunked = Mock(
return_value=[
b'{"type": "ADDED", "object": {"metadata": {"name": "test1",'
b'"resourceVersion": "1"}, "spec": {}, "status": {}}}\n',
b'{"type": "ADDED", "object": {"metadata": {"name": "test2",'
b'"resourceVersion": "2"}, "spec": {}, "sta',
b'tus": {}}}\n'
b'{"type": "ADDED", "object": {"metadata": {"name": "test3",'
b'"resourceVersion": "3"}, "spec": {}, "status": {}}}\n',
b'should_not_happened\n'])
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused. Does the Python 3 client always get byte formatted data in a watch response? If so, does that mean the Python 3 client never supported watch?

I'm not sure if I understand what triggers a byte formatted response from the apiserver. Could you add an e2e test in https://github.com/kubernetes-client/python/blob/master/kubernetes/e2e_test/test_client.py for watch, so that we can see watch works for both Python 2 and 3?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, but it seems to me that yes, p3 always gets bytes. I'm not sure about 'never supported watch', I may guess that it's not working only if data contains non-ascii symbols.

Writing e2e test is a great idea, but I'm not quiet sure I understand how watch test should look like. If you could help me out with describing test case, I'd do it faster :)

In testcase test_watch I'm creating, let's say, configmap, remember it's revision, make changes to this configmap, and start watching it from saved revision, sounds legit?


fake_api = Mock()
fake_api.get_namespaces = Mock(return_value=fake_resp)
Expand Down Expand Up @@ -172,6 +184,9 @@ def test_watch_stream_twice(self):
fake_resp.release_conn = Mock()
fake_resp.read_chunked = Mock(
return_value=['{"type": "ADDED", "object": 1}\n'] * 4)
if six.PY3:
fake_resp.read_chunked = Mock(
return_value=[b'{"type": "ADDED", "object": 1}\n'] * 4)

fake_api = Mock()
fake_api.get_namespaces = Mock(return_value=fake_resp)
Expand Down Expand Up @@ -199,6 +214,9 @@ def test_watch_stream_loop(self):
fake_resp.release_conn = Mock()
fake_resp.read_chunked = Mock(
return_value=['{"type": "ADDED", "object": 1}\n'])
if six.PY3:
fake_resp.read_chunked = Mock(
return_value=[b'{"type": "ADDED", "object": 1}\n'])

fake_api = Mock()
fake_api.get_namespaces = Mock(return_value=fake_resp)
Expand Down