<div dir="ltr"><div><div><div>Hi Piotr,<br><br></div>Yes, I saw Nir's comment on the gerrit.<br></div>I'll leave my hook doing "import hooking" then.<br><br></div>Thanks a lot for the help.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 28, 2017 at 6:29 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"><span class="">On Mon, Nov 27, 2017 at 11:56 PM, Germano Veit Michel<br>
<<a href="mailto:germano@redhat.com">germano@redhat.com</a>> wrote:<br>
> Hi Piotr and Nir,<br>
><br>
> Thanks for following up and sorry for sending to the wrong email list.<br>
><br>
> So is the conclusion that 84638 will be merged and all hooks will just<br>
> "import hooking"?<br>
<br>
</span>It is not easy to change it so we need to keep above import. I haven't<br>
seen any disagreement so I<br>
hope that the patch will be merged soon to enable you to write hook tests.<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Thanks,<br>
><br>
><br>
><br>
> On Mon, Nov 27, 2017 at 6:32 PM, Piotr Kliczewski<br>
> <<a href="mailto:piotr.kliczewski@gmail.com">piotr.kliczewski@gmail.com</a>> wrote:<br>
>><br>
>> 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>><br>
>> >> > 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<br>
>> >> >>> 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<br>
>> >> >>> need<br>
>> >> >>> vdsm<br>
>> >> >>> installed on my development machine in order for this import to<br>
>> >> >>> work.<br>
>> >> >>><br>
>> >> >>><br>
>> >> >>> ==============================<wbr>==============================<wbr>==========<br>
>> >> >>> ERROR: test suite for <class<br>
>> >> >>> 'virttests.boot_hostdev_test.<wbr>BootHostdevHookTests'><br>
>> >> >>><br>
>> >> >>> ------------------------------<wbr>------------------------------<wbr>----------<br>
>> >> >>> Traceback (most recent call last):<br>
>> >> >>> File "/usr/lib/python2.7/site-<wbr>packages/nose/suite.py", line 209,<br>
>> >> >>> in<br>
>> >> >>> run<br>
>> >> >>> self.setUp()<br>
>> >> >>> File "/usr/lib/python2.7/site-<wbr>packages/nose/suite.py", line 292,<br>
>> >> >>> in<br>
>> >> >>> setUp<br>
>> >> >>> self.setupContext(ancestor)<br>
>> >> >>> File "/usr/lib/python2.7/site-<wbr>packages/nose/suite.py", line 315,<br>
>> >> >>> in<br>
>> >> >>> setupContext<br>
>> >> >>> try_run(context, names)<br>
>> >> >>> File "/usr/lib/python2.7/site-<wbr>packages/nose/util.py", line 471,<br>
>> >> >>> in<br>
>> >> >>> try_run<br>
>> >> >>> return func()<br>
>> >> >>> File<br>
>> >> >>><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<br>
>> >> >>> the<br>
>> >> >>> correct way to do this import?<br>
>> >> ><br>
>> >> > Our approach for it is to modify PYTHONPATH [1] where we add<br>
>> >> > 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>
>> I agree that the path is not perfect way of doing it. I would love to fix<br>
>> 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<br>
>> 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<br>
>> path.<br>
>><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>
>> >> ><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>
><br>
><br>
><br>
><br>
> --<br>
> Germano Veit Michel <<a href="mailto:germano@redhat.com">germano@redhat.com</a>><br>
> Senior Software Maintenance Engineer, Virtualization, Red Hat<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>