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