Change in vdsm[master]: Introduce commands package to separate execCmd and AsyncProc...

nsoffer at redhat.com nsoffer at redhat.com
Fri Jan 8 14:15:42 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: Introduce commands package to separate execCmd and AsyncProc from utils
......................................................................


Patch Set 4:

(15 comments)

Nice! but you move too much stuff for no reason.

We need to move execCmd and anything that depends on it to new commands module.

All the users of utils.execCmd and friends should get them from new commands module. These module cannot use Python 3 until we reimplment execCmd in a way that works with Python 3.

commands module should import utils for generic stuff like @stopwatch

All the code that import utils but does not depend on execCmd and friends can be ported now to Python 3.

https://gerrit.ovirt.org/#/c/51421/4/lib/vdsm/commands.py
File lib/vdsm/commands.py:

Line 471: 
Line 472:             self._poll_timeout(poll_remaining)
Line 473: 
Line 474: 
Line 475: def monotonic_time():
Why move this here? this can stay in utils.
Line 476:     """
Line 477:     Return the amount of time, in secs, elapsed since a fixed
Line 478:     arbitrary point in time in the past.
Line 479:     This function is useful if the client just


Line 489:     """
Line 490:     return os.times()[4]
Line 491: 
Line 492: 
Line 493: class GeneralException(Exception):
Why move this here?
Line 494:     code = 100
Line 495:     message = "General Exception"
Line 496: 
Line 497:     def __init__(self, *value):


Line 536: 
Line 537:     return proc.returncode, out, err
Line 538: 
Line 539: 
Line 540: def retry(func, expectedException=Exception, tries=None,
Why move this?
Line 541:           timeout=None, sleep=1, stopCallback=None):
Line 542:     """
Line 543:     Retry a function. Wraps the retry logic so you don't have to
Line 544:     implement it each time you need it.


Line 580:             time.sleep(sleep)
Line 581: 
Line 582: 
Line 583: @contextmanager
Line 584: def stopwatch(message, log=logging.getLogger('vds.stopwatch')):
Why move this?
Line 585:     if log.isEnabledFor(logging.DEBUG):
Line 586:         start = monotonic_time()
Line 587:         yield
Line 588:         elapsed = monotonic_time() - start


https://gerrit.ovirt.org/#/c/51421/4/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 87
Line 88
Line 89
Line 90
Line 91
Do not move, commands will import it from utils.


Line 104
Line 105
Line 106
Line 107
Line 108
Do not move, commands will import this from utils.


Line 169
Line 170
Line 171
Line 172
Line 173
Move to host.py?


Line 305
Line 306
Line 307
Line 308
Line 309
Do not move, commands will import this from utils.


Line 327
Line 328
Line 329
Line 330
Line 331
Do not move, commands will import this from utils.


Line 342
Line 343
Line 344
Line 345
Line 346
This does not depend on execCmd (it receive a Popen object, does not case how you created it), and should be compatible with Python 3, so keep it here for now.


Line 671
Line 672
Line 673
Line 674
Line 675
Do not move, commands will import this from utils, in case other code would like to use this.


Line 765
Line 766
Line 767
Line 768
Line 769
This should move into new host.py module, providing host.uuid() function.

Later we can move there more stuff that return info about this host.


Line 904
Line 905
Line 906
Line 907
Line 908
Do not move, commands will import this from utils.


Line 1184
Line 1185
Line 1186
Line 1187
Line 1188
Do not move, commands will import this from utils.


Line 1232
Line 1233
Line 1234
Line 1235
Line 1236
Do not move, commands will import this from utils.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I295f5516154c464fe5b1fc103b7b9d1d76dffe17
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.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