Change in vdsm[master]: bridge: obtain method

nsoffer at redhat.com nsoffer at redhat.com
Fri Mar 11 18:41:49 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: bridge: obtain method
......................................................................


Patch Set 12:

(6 comments)

Looks good, need to fix the filtered methods set.

https://gerrit.ovirt.org/#/c/53472/12/lib/yajsonrpc/__init__.py
File lib/yajsonrpc/__init__.py:

Line 470
Line 471
Line 472
Line 473
Line 474
We don't mangle the names now, so these names should be updated.


Line 506
Line 507
Line 508
Line 509
Line 510
In a later patch I would change the logLevel like this:

    if req.method in self.FILTERED_METHODS:
        logLevel = ...
    else:
        logLevel = ...

Little easier to follow like this


Line 527
Line 528
Line 529
Line 530
Line 531
For another patch, if the request fails, we don't unregister server address


Line 528
Line 529
Line 530
Line 531
Line 532
For another patch, we should log exceptions here.


Line 535
Line 536
Line 537
Line 538
Line 539
for another patch, we don't need this else


Line 514:         except JsonRpcMethodNotFoundError as e:
Line 515:             if req.isNotification():
Line 516:                 return
Line 517: 
Line 518:             ctx.requestDone(JsonRpcResponse(None, e, req.id))
For another patch, we should have separate types for successful and error responses:

    JsonRpcSuccessResponse(value, id)
    JsonRpcErrorResponse(error, id)
Line 519:             return
Line 520: 
Line 521:         try:
Line 522:             params = req.params


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b6cb53d8e42335510e34dfe372c1c21b80ac219
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list