[GitHub] [incubator-heron] Code0x58 commented on a change in pull request #3574: Decode string when deserializing Java object in python
Code0x58 commented on a change in pull request #3574: URL: https://github.com/apache/incubator-heron/pull/3574#discussion_r457352071 ## File path: heron/statemgrs/src/python/zkstatemanager.py ## @@ -170,7 +170,7 @@ def _get_topology_with_watch(self, topologyName, callback, isWatching): # pylint: disable=unused-variable, unused-argument @self.client.DataWatch(path) -def watch_topology(data, stats): +def watch_topology(data, stats, event): """ watch topology """ if data: topology = Topology() Review comment: I think it helps by avoiding the questionable pattern [here](https://github.com/python-zk/kazoo/blob/master/kazoo/recipe/watchers.py#L161) which can obscure the exceptions when they occur within the handler - at least we certainly won't see the TypeError now even if the parameter is junk (no more "exception occurred while handling exception", so that much easier to read) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on a change in pull request #3574: Decode string when deserializing Java object in python
Code0x58 commented on a change in pull request #3574: URL: https://github.com/apache/incubator-heron/pull/3574#discussion_r457352071 ## File path: heron/statemgrs/src/python/zkstatemanager.py ## @@ -170,7 +170,7 @@ def _get_topology_with_watch(self, topologyName, callback, isWatching): # pylint: disable=unused-variable, unused-argument @self.client.DataWatch(path) -def watch_topology(data, stats): +def watch_topology(data, stats, event): """ watch topology """ if data: topology = Topology() Review comment: I think it helps by avoiding the questionable pattern [here](https://github.com/python-zk/kazoo/blob/master/kazoo/recipe/watchers.py#L161) which can obscure the exceptions when they occur within the handler - at least we certainly won't see the TypeError now even if the parameter is junk This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on a change in pull request #3574: Decode string when deserializing Java object in python
Code0x58 commented on a change in pull request #3574: URL: https://github.com/apache/incubator-heron/pull/3574#discussion_r457352071 ## File path: heron/statemgrs/src/python/zkstatemanager.py ## @@ -170,7 +170,7 @@ def _get_topology_with_watch(self, topologyName, callback, isWatching): # pylint: disable=unused-variable, unused-argument @self.client.DataWatch(path) -def watch_topology(data, stats): +def watch_topology(data, stats, event): """ watch topology """ if data: topology = Topology() Review comment: I think it helps by avoiding the questionable pattern [here](https://github.com/python-zk/kazoo/blob/master/kazoo/recipe/watchers.py#L161) which might be having some weird interaction with the rest of the codebase - at least we certainly won't see the TypeError now even if the parameter is junk This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [incubator-heron] Code0x58 commented on a change in pull request #3574: Decode string when deserializing Java object in python
Code0x58 commented on a change in pull request #3574: URL: https://github.com/apache/incubator-heron/pull/3574#discussion_r457352071 ## File path: heron/statemgrs/src/python/zkstatemanager.py ## @@ -170,7 +170,7 @@ def _get_topology_with_watch(self, topologyName, callback, isWatching): # pylint: disable=unused-variable, unused-argument @self.client.DataWatch(path) -def watch_topology(data, stats): +def watch_topology(data, stats, event): """ watch topology """ if data: topology = Topology() Review comment: I think it helps by avoiding the questionable pattern [here](https://github.com/python-zk/kazoo/blob/master/kazoo/recipe/watchers.py#L155) which might be having some weird interaction with the rest of the codebase - at least we certainly won't see the TypeError now even if the parameter is junk This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
