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