Nir Soffer has posted comments on this change.

Change subject: storage: Move mount module to vdsm storage lib.
......................................................................


Patch Set 11: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/55292/11/tests/mkimageTests.py
File tests/mkimageTests.py:

Line 35: from monkeypatch import Patch
Line 36: from testlib import VdsmTestCase, permutations, expandPermutations
Line 37: from testValidation import checkSudo, ValidateRunningAsRoot
Line 38: 
Line 39: from vdsm import storage
> I think would be safer to import mount directly, because some code might st
Good catch!

This import was actually wrong in the original code - it works only because 
some other code did:

    import mount

Or:

    from storage import mount

Which added a "mount" attribute to the "storage" module in sys.modules.

We should fix this in the original code *before* moving it, lets not mix code 
movments and other changes in the same patch.

So lets have another patch before this, changing code using:

    import storage
    ...
    storage.mount.XXX

To:

    from storage import mount
    mount.XXX
Line 40: from vdsm import udevadm
Line 41: from vdsm.commands import execCmd
Line 42: from vdsm.utils import stopwatch
Line 43: import mkimage


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ee9fab87abcabc469c15256ba12386112a100e0
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to