Change in vdsm[master]: stomp: outgoing connection to a broker
ybronhei at redhat.com
ybronhei at redhat.com
Wed May 13 14:36:40 UTC 2015
Yaniv Bronhaim has posted comments on this change.
Change subject: stomp: outgoing connection to a broker
......................................................................
Patch Set 13: Code-Review-1
(9 comments)
https://gerrit.ovirt.org/#/c/38819/13/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:
Line 180: 'Enable HTTP/1.1 keep-alive connections'),
Line 181:
Line 182: ('jsonrpc_enable', 'true', 'Enable the JSON RPC server'),
Line 183:
Line 184: ('broker_enable', 'false', 'Enable outgoing connection to broker'),
mention in the commit message what values you add to vdsm config and the reason for the defaults
Line 185:
Line 186: ('report_host_threads_as_cores', 'false',
Line 187: 'Count each cpu hyperthread as an individual core'),
Line 188:
Line 323: 'Port where the broker is listening at.'),
Line 324:
Line 325: ('request_queues',
Line 326: 'jms.topic.vdsm_requests,jms.topic.vdsm_irs_requests',
Line 327: 'Queues to subscribe to for RPC requests'),
to or for?
what is jms?
Line 328:
Line 329: ('event_queue', 'jms.topic.vdsm_responses',
Line 330: 'Queue used for events'),
Line 331: ]),
Line 326: 'jms.topic.vdsm_requests,jms.topic.vdsm_irs_requests',
Line 327: 'Queues to subscribe to for RPC requests'),
Line 328:
Line 329: ('event_queue', 'jms.topic.vdsm_responses',
Line 330: 'Queue used for events'),
jsonrpc events
Line 331: ]),
Line 332:
Line 333: # Section: [sampling]
Line 334: ('sampling', [
https://gerrit.ovirt.org/#/c/38819/13/lib/yajsonrpc/stompReactor.py
File lib/yajsonrpc/stompReactor.py:
Line 162: return
Line 163: elif stomp.LEGACY_SUB_ID_REQ == destination:
Line 164: self._handle_internal(dispatcher, stomp.LEGACY_SUB_ID_RES,
Line 165: frame.body)
Line 166: return
no else? could it cause keyerror next line?
Line 167:
Line 168: subs = self._sub_dests[destination]
Line 169: if not subs:
Line 170: self._send_error("Subscription not available",
Line 256:
Line 257: class StompServer(object):
Line 258: log = logging.getLogger("yajsonrpc.StompServer")
Line 259:
Line 260: def __init__(self, reactor, subs):
why do you do all those words shortcuts? call it subscriptions please
req can easily refer to request, but when you also have res req sub its quite annoying in a code that should be readable imo.
Line 261: self._reactor = reactor
Line 262: self._messageHandler = None
Line 263: self._sub_map = subs
Line 264: self._req_dest = {}
Line 349:
Line 350: def send(
Line 351: self,
Line 352: message,
Line 353: destination=stomp.LEGACY_SUB_ID_RES,
LEGACY_SUBSCRIPTION_ID_RESPONSE will fit in line
Line 354: headers=None
Line 355: ):
Line 356: self.log.debug("Sending request")
Line 357: self._aclient.send(
Line 436: self.json_binding.add_socket(self._reactor, client_socket)
Line 437: self.log.debug("Stomp detected from %s", socket_address)
Line 438:
Line 439:
Line 440: class ServerRpcContextAdapter(object):
please explain the need for this class . easy to understand but a comment should help readers.
Line 441: @classmethod
Line 442: def subscription_handler(cls, server):
Line 443: def handler(sub, frame):
Line 444: server.queueRequest(
https://gerrit.ovirt.org/#/c/38819/13/vdsm/clientIF.py
File vdsm/clientIF.py:
Line 91: self._generationID = str(uuid.uuid4())
Line 92: self.mom = None
Line 93: self.bindings = {}
Line 94: self._broker_client = None
Line 95: self.subs = defaultdict(list)
self.subscriptions please
Line 96: if _glusterEnabled:
Line 97: self.gluster = gapi.GlusterApi(self, log)
Line 98: else:
Line 99: self.gluster = None
Line 194:
Line 195: self._acceptor = MultiProtocolAcceptor(self._reactor, host,
Line 196: port, sslctx)
Line 197:
Line 198: def _connectToBroker(self):
who calls it?
Line 199: if config.getboolean('vars', 'broker_enable'):
Line 200: broker_address = config.get('addresses', 'broker_address')
Line 201: broker_port = config.getint('addresses', 'broker_port')
Line 202: request_queues = config.get('addresses', 'request_queues')
--
To view, visit https://gerrit.ovirt.org/38819
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied3095a305fd5a5bfc19c8bc0342fba7cb811843
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list