Nir Soffer has posted comments on this change.

Change subject: jsonrpc: introduce new client
......................................................................


Patch Set 10:

(1 comment)

https://gerrit.ovirt.org/#/c/64502/10/lib/vdsm/client.py
File lib/vdsm/client.py:

Line 108: ServerError will be raised if the command encountered execution error:
Line 109: 
Line 110:     vdsm.client.ServerError: Vdsm request failed
Line 111:     (code=4, message=Virtual machine already exists)
Line 112: 
> you repeat the errors description twice (here and in the call method). I gu
Yaniv, please see this:
http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments
Line 113: 
Line 114: Please note that if the client isn't a context manager, it should be
Line 115: closed at the end of the run:
Line 116: 


-- 
To view, visit https://gerrit.ovirt.org/64502
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idd45d7e88bf2246beaf30550b12201917f32c354
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoih...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Irit Goihman <igoih...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák <msi...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Reply via email to