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

quick fix of decoding error for BOOKMARK event #234

Merged
merged 1 commit into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion watch/watch.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ def get_watch_argument_name(self, func):
def unmarshal_event(self, data, return_type):
js = json.loads(data)
js['raw_object'] = js['object']
if return_type and js['type'] != 'ERROR':
# BOOKMARK event is treated the same as ERROR for a quick fix of
# decoding exception
# TODO: make use of the resource_version in BOOKMARK event for more
# efficient WATCH
if return_type and js['type'] != 'ERROR' and js['type'] != 'BOOKMARK':
obj = SimpleNamespace(data=json.dumps(js['raw_object']))
js['object'] = self._api_client.deserialize(obj, return_type)
if hasattr(js['object'], 'metadata'):
Expand Down
13 changes: 13 additions & 0 deletions watch/watch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,19 @@ def test_unmarshal_with_custom_object(self):
self.assertEqual("1", event['object']['metadata']['resourceVersion'])
self.assertEqual("1", w.resource_version)

def test_unmarshal_with_bookmark(self):
w = Watch()
event = w.unmarshal_event(
'{"type":"BOOKMARK","object":{"kind":"Job","apiVersion":"batch/v1"'
',"metadata":{"resourceVersion":"1"},"spec":{"template":{'
'"metadata":{},"spec":{"containers":null}}},"status":{}}}',
Copy link
Member

Choose a reason for hiding this comment

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

nit: reading the doc, it looks like a bookmark event won't contain things like job spec and job status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's correct, the spec and status are all empty, even though they seem to contain something.

Copy link
Member

Choose a reason for hiding this comment

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

I thought the key would be omitted, but maybe that's the wrong impression the doc gave me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is not. kubernetes-client/python#1404 has the BOOKMARK event:

{"type":"BOOKMARK","object":{"kind":"Job","apiVersion":"batch/v1","metadata":{"resourceVersion":"45398385","creationTimestamp":null},"spec":{"template":{"metadata":{"creationTimestamp":null},"spec":{"containers":null}}},"status":{}}}

'V1Job')
self.assertEqual("BOOKMARK", event['type'])
# Watch.resource_version is *not* updated, as BOOKMARK is treated the
# same as ERROR for a quick fix of decoding exception,
# resource_version in BOOKMARK is *not* used at all.
self.assertEqual(None, w.resource_version)

def test_watch_with_exception(self):
fake_resp = Mock()
fake_resp.close = Mock()
Expand Down