Zhou Zheng Sheng has posted comments on this change. Change subject: Refactoring storage.misc: moving watchCmd and its deps to vdsm.utils. ......................................................................
Patch Set 16: I would prefer that you didn't submit this (2 inline comments) Some modules still refer to the old place of ActionStopped and stripNewLines, I mark them in the comments. I'm sorry for not verifying them in previous review. As we move the related things from misc to utils, we find some problems such as the logger's name and the exceptions. I can accept the solution for logging now, we can improve it in future patches. As regard to exception classes, the GeneralException and ActionStopped are moved to utils as well, but these two are storage specific exceptions, you can see there are fields like code and message in them like other storage exceptions. Having these two exceptions in utils maybe a bad idea because it mixes storage specific concepts in utils even though the utils code does not depend on storage code. I think a better way is that define an exception named "Stopped" in utils and let utils.watchCmd throw it, then define another watchCmd in misc to invoke utils.watchCmd, except the "Stopped", change it to storage_exception.ActionStopped and re-raise it. The above are just my thoughts, I -1 for not updating all the reference to the moved classes. .................................................... File vdsm/storage/misc.py Line 72 Line 73 Line 74 Line 75 Line 76 The vdsm/storage/fileSD.py still refer to misc.stripNewLines not vdsm.utils.stripNewLines . .................................................... File vdsm/storage/storage_exception.py Line 1017 Line 1018 Line 1019 Line 1020 Line 1021 Modules still have references to storage_exception.ActionStopped are as follow vdsm/storage/fileVolume.py vdsm/storage/blockVolume.py vdsm/storage/image.py vdsm/storage/blockSD.py I think you can either change those files to use vdsm.utils.ActionStopped, or "from vdsm.utils import ActionStopped" in storage_exception.py . -- To view, visit http://gerrit.ovirt.org/14408 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I45eaf487b0260863af09087e5114fb836276a750 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Zhou Zheng Sheng <zhshz...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
