On Mon, Nov 27, 2017 at 11:56 PM, Germano Veit Michel <[email protected]> wrote: > Hi Piotr and Nir, > > Thanks for following up and sorry for sending to the wrong email list. > > So is the conclusion that 84638 will be merged and all hooks will just > "import hooking"?
It is not easy to change it so we need to keep above import. I haven't seen any disagreement so I hope that the patch will be merged soon to enable you to write hook tests. > > Thanks, > > > > On Mon, Nov 27, 2017 at 6:32 PM, Piotr Kliczewski > <[email protected]> wrote: >> >> On Sat, Nov 25, 2017 at 1:56 AM, Nir Soffer <[email protected]> wrote: >> > On Fri, Nov 24, 2017 at 5:34 PM Piotr Kliczewski >> > <[email protected]> wrote: >> >> >> >> On Fri, Nov 24, 2017 at 4:00 PM, Piotr Kliczewski >> >> <[email protected]> wrote: >> >> > On Fri, Nov 24, 2017 at 2:20 PM, Nir Soffer <[email protected]> >> >> > wrote: >> >> >> Adding devel@ovirt, the proper mailing list >> >> >> >> >> >> בתאריך יום ו׳, 24 בנוב׳ 2017, 7:42, מאת Germano Veit Michel >> >> >> <[email protected]>: >> >> >>> >> >> >>> Hi, >> >> >>> >> >> >>> I'm trying to write a test for a hook. The test will fail if the >> >> >>> hook >> >> >>> does >> >> >>> "import hooking", which seems to be the norm for vdsm hooks. >> >> >>> >> >> >>> [vdsm_hooks]$ grep -rn "import hooking" | wc -l >> >> >>> 55 >> >> >>> >> >> >>> Not sure if I'm doing something wrong, but it looks like I would >> >> >>> need >> >> >>> vdsm >> >> >>> installed on my development machine in order for this import to >> >> >>> work. >> >> >>> >> >> >>> >> >> >>> ====================================================================== >> >> >>> ERROR: test suite for <class >> >> >>> 'virttests.boot_hostdev_test.BootHostdevHookTests'> >> >> >>> >> >> >>> ---------------------------------------------------------------------- >> >> >>> Traceback (most recent call last): >> >> >>> File "/usr/lib/python2.7/site-packages/nose/suite.py", line 209, >> >> >>> in >> >> >>> run >> >> >>> self.setUp() >> >> >>> File "/usr/lib/python2.7/site-packages/nose/suite.py", line 292, >> >> >>> in >> >> >>> setUp >> >> >>> self.setupContext(ancestor) >> >> >>> File "/usr/lib/python2.7/site-packages/nose/suite.py", line 315, >> >> >>> in >> >> >>> setupContext >> >> >>> try_run(context, names) >> >> >>> File "/usr/lib/python2.7/site-packages/nose/util.py", line 471, >> >> >>> in >> >> >>> try_run >> >> >>> return func() >> >> >>> File >> >> >>> >> >> >>> >> >> >>> "/home/gveitmic/Source/upstream/vdsm/tests/virttests/boot_hostdev_test.py", >> >> >>> line 127, in setUpClass >> >> >>> import before_vm_start as boot_hostdev_hook >> >> >>> File "../vdsm_hooks/boot_hostdev/before_vm_start.py", line 24, in >> >> >>> <module> >> >> >>> import hooking >> >> >>> ImportError: No module named hooking >> > >> > >> > This is expected, hooking is not install in the pyhton path by default. >> > >> > All the hooks are using wrong import. Vdsm is "fixing" the bad >> > import by modifying the python path. >> > >> >> >> >> >>> >> >> >>> I can make it go away by using this in my hook: >> >> >>> >> >> >>> from vdsm.hook import hooking >> > >> > >> > This is correct import - I would use this. >> > >> >> >> >> >>> >> >> >>> Then the test succeeds fine. But then I see all hooks use "import >> >> >>> hooking" >> >> >>> instead of "from vdsm.hook import hooking". >> >> >>> >> >> >>> [vdsm_hooks]$ grep -rn "from vdsm.hook import hooking" | wc -l >> >> >>> 0 >> >> >>> >> >> >>> Am I doing something wrong? Can someone put a light here? What is >> >> >>> the >> >> >>> correct way to do this import? >> >> > >> >> > Our approach for it is to modify PYTHONPATH [1] where we add >> >> > vdsm.hook. >> >> > It looks like we do not have similar update in [2] - script which run >> >> > the tests. >> >> > >> >> > I will push a patch to fix it. >> >> >> >> https://gerrit.ovirt.org/#/c/84638/ >> >> >> >> Please check whether it would work for you now. >> > >> > >> > This can be handy for testing legacy hooks, but I think we should >> > fix the imports instead, so no manipulation of python path is needed. >> > >> >> I agree that the path is not perfect way of doing it. I would love to fix >> it but >> please note that people use `import hooking` in their own custom hooks >> and we are unable to change them. This means that we won't be able to >> fix it easily if ever. Because of that I am not sure whether it is good >> idea to >> introduce different way of importing the module. >> >> Till now we haven't had hook tests so the module was not available on the >> path. >> >> >> >> >> >> >> > >> >> > [1] https://github.com/oVirt/vdsm/blob/master/lib/vdsm/hooks.py#L98 >> >> > [2] >> >> > >> >> > https://github.com/oVirt/vdsm/blob/master/tests/run_tests_local.sh.in#L21 >> >> > >> >> >>> >> >> >>> Thanks, >> >> >>> Germano >> >> >> >> >> >> >> >> >> _______________________________________________ >> >> >> Devel mailing list >> >> >> [email protected] >> >> >> http://lists.ovirt.org/mailman/listinfo/devel > > > > > -- > Germano Veit Michel <[email protected]> > Senior Software Maintenance Engineer, Virtualization, Red Hat _______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
