Change in vdsm[master]: contrib: Simple jsonrpc client

piotr.kliczewski at gmail.com piotr.kliczewski at gmail.com
Wed Dec 16 13:57:08 UTC 2015


Piotr Kliczewski has posted comments on this change.

Change subject: contrib: Simple jsonrpc client
......................................................................


Patch Set 6:

(7 comments)

I am happy to take over. I am adding it to my TODO queue.

https://gerrit.ovirt.org/#/c/35181/6/contrib/jsonrpc
File contrib/jsonrpc:

Line 60: 
Line 61: """
Line 62: 
Line 63: import json
Line 64: import optparse
> Better use argparse for new code, it gives better online help.
Done
Line 65: import os
Line 66: import ssl
Line 67: import sys
Line 68: import warnings


Line 65: import os
Line 66: import ssl
Line 67: import sys
Line 68: import warnings
Line 69: from uuid import uuid4
> Don't import functions from modules, only modules.
Done
Line 70: 
Line 71: from yajsonrpc import stompreactor, JsonRpcRequest
Line 72: from vdsm.sslutils import SSLContext
Line 73: 


Line 68: import warnings
Line 69: from uuid import uuid4
Line 70: 
Line 71: from yajsonrpc import stompreactor, JsonRpcRequest
Line 72: from vdsm.sslutils import SSLContext
> We need simpler way to create and use a client.
Done
Line 73: 
Line 74: 
Line 75: # Suppress unhelpful warnings
Line 76: warnings.simplefilter("ignore", DeprecationWarning)


Line 78: # Copied from lib/vdsm/vdscli.py
Line 79: PKIDIR = '/etc/pki/vdsm'
Line 80: KEYFILE = os.path.join(PKIDIR, 'keys/vdsmkey.pem')
Line 81: CERTFILE = os.path.join(PKIDIR, 'certs/vdsmcert.pem')
Line 82: CACERT = os.path.join(PKIDIR, 'certs/cacert.pem')
> This should be hidden inside vdsm client when you run on local machine, the
Done
Line 83: 
Line 84: PORT = 54321
Line 85: DESTINATION = "jms.topic.vdsm_requests"
Line 86: 


Line 109:             req = JsonRpcRequest(method, request_params, reqId=str(uuid4()))
Line 110:             # we need to make sure that waiting on response can be
Line 111:             # interrupted using keyboard
Line 112:             responses = client.call(req)
Line 113:             if responses:
> Why if we got no response?  is this possible? We probably need to fail loud
If there are issues with connection and we timeout there is no response. I agree with the failure.
Line 114:                 # we may want to implement nice __str__() or __repr__()
Line 115:                 # it is not json so there is no need to use prettify
Line 116:                 print responses[0]
Line 117:         finally:


Line 112:             responses = client.call(req)
Line 113:             if responses:
Line 114:                 # we may want to implement nice __str__() or __repr__()
Line 115:                 # it is not json so there is no need to use prettify
Line 116:                 print responses[0]
> We want here prety printed json - I guess we don't send nicely indented jso
Will work on it
Line 117:         finally:
Line 118:             client.close()
Line 119:     # improve exception handling
Line 120:     except:


Line 165:     sslctx = SSLContext(key_file=KEYFILE, cert_file=CERTFILE,
Line 166:                         ca_certs=CACERT, protocol=ssl.PROTOCOL_TLSv1)
Line 167: 
Line 168:     return stompreactor.StandAloneRpcClient(options.host, PORT,
Line 169:                                             'jms.topic.vdsm_requests',
> Use DESTINATION
This desired approach to use something unique for response queue to limit collisions. In this way we can have as many clients running in parallel as we want.
Line 170:                                             str(uuid4()), sslctx,
Line 171:                                             lazy_start=False)
Line 172: 
Line 173: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6273eabf6f3601602659d1e4e748d8025ae8084
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni+ovirt at midokura.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list