Zhou Zheng Sheng has posted comments on this change.
Change subject: Logging: config logging in new process of remoteFileHandler
......................................................................
Patch Set 3: (1 inline comment)
Thanks Dan. I was confused with the comments you notice. Now I check the code
and think those comments and the 'if' protection can be removed now.
....................................................
File vdsm/storage/remoteFileHandler.py
Line 33:
Line 34: import misc
Line 35: import fileUtils
Line 36: import zombieReaper
Line 37: from vdsm import constants
I think the comment can be removed now, and the import statements can be moved
outside of the 'if __name__ != "__main__":' block.
In production system
The comment says if you don't have the vdsm package installed this will fail.
However remoteFileHandler.py is in the vdsm package. How do we get this file
without installing vdsm package? Maybe the comment means if the vdsm-python is
not installed, the import will fail. However vdsm-python is dependency of vdsm
package. How can we install vdsm without vdsm-python?
In unit test in the source workspace:
When this file is loaded for unit test, the working dir is ./tests. The
hackVdsmModule() in testrunner.py sets sys.modules['vdsm.constants'].P_VDSM to
"../". Then in PoolHandler.__init__(), the PYTHONPATH for the newly spawned
child is set to constants.P_VDSM:env.get("PYTHONPATH", "") , which means "../"
is prepend to PYTHONPATH env. So when the child executes "from vdsm import
constants", it finds the vdsm module under the local source tree under the
./tests/../vdsm/__init__.py, then it get constant module from
./tests/../vdsm/constants.py .
This trick is introduced in commit d8778adb1d4, reviewed at
http://gerrit.ovirt.org/9217 .
So it's OK for this file to "from vdsm import constants" as well.
So as I can see, when python loads this file, it can import modules under vdsm
both from the vdsm daemon process, the unit test process and the newly spawned
child process.
Line 38:
Line 39: # Crabs are known for their remote process calls
Line 40: LENGTH_STRUCT_FMT = "Q"
Line 41: LENGTH_STRUCT_LENGTH = calcsize(LENGTH_STRUCT_FMT)
--
To view, visit http://gerrit.ovirt.org/9182
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b3b9e2837d632c7532fcd8f7306ed50b0865b5c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Zhou Zheng Sheng <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
