Francesco Romani has posted comments on this change. Change subject: jsonrpc: events ......................................................................
Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/38069/10/vdsm/clientIF.py File vdsm/clientIF.py: Line 145: Line 146: def _send_notification(self, message, destination): Line 147: json_binding = self.bindings['jsonrpc'] Line 148: server = json_binding.getStompReactor().server Line 149: server.send(message) We have two problems here. One is that we need to jumps through a few hoops, most notably keep ClientIF references around, just to send notification. Moreover, ClientIF doesn't fill the right place for notifications. But let's save this for a later moment The second is that I'm under the impression that most often client code just wants to do one notification and forget about it. No need to keep around state, thus no need to have a create_* (and keep some state around) So, a function like def notify(self, event_id, **kwargs): event = Notification(event_id, self._send_notification) event.emit(**kwargs) to be used like (see https://gerrit.ovirt.org/#/c/38937/1/vdsm/virt/vm.py) # this is inside Vm class self.cif.notify(self.id='UP') seems bot nicer to use and shorter to implement. What do you think? Line 150: Line 151: def contEIOVms(self, sdUUID, isDomainStateValid): Line 152: # This method is called everytime the onDomainStateChange Line 153: # event is emitted, this event is emitted even when a domain goes -- To view, visit https://gerrit.ovirt.org/38069 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id27b5ca1773139932eb5cb16921d5abec4991c5e Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Dima Kuznetsov <dkuzn...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
