Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(2 comments)
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN) Line 344: Line 345: def _epoll_input(self, fileno):
This method is handling the poll input event, the name seems consistent.
These names are consistent, but they are not good method names. They do not describe what the method does, and do not help readability.
The method that does polling is not the same thing as the method that handle certain event, so their names should not be "consistent".
Having epoll in the name adds no value, because we don't have several method of polling, one with epoll and one without another type. It would be better if we can replace epoll with poll without modifying these names.
I would expect to have names like:
- _poll - wait for events. This is typically done with a timeout, and we don't have another method for polling without a timeouts, so the _timeout suffix is not needed. - _handle_input - handle input from a watched fd - _handle_close - handle closed fd Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno)
Line 363: return len(self.iocb) == 0 Line 364: Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1
If we do it in the loop it will be set at every cycle unless we rely on som
I'm not worried about setting this each iteration. Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed: