Hi Piotr, Yes, I saw Nir's comment on the gerrit. I'll leave my hook doing "import hooking" then.
Thanks a lot for the help. On Tue, Nov 28, 2017 at 6:29 PM, Piotr Kliczewski < [email protected]> wrote: > 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 > -- Germano Veit Michel <[email protected]> Senior Software Maintenance Engineer, Virtualization, Red Hat
_______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
