[ovirt-devel] [rhev-tech] Correct way to import hooking?

[Piotr Kliczewski]

On Mon, Nov 27, 2017 at 11:56 PM, Germano Veit Michel
<germano at redhat.com> 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
> <piotr.kliczewski at gmail.com> wrote:
>>
>> On Sat, Nov 25, 2017 at 1:56 AM, Nir Soffer <nsoffer at redhat.com> wrote:
>> > On Fri, Nov 24, 2017 at 5:34 PM Piotr Kliczewski
>> > <piotr.kliczewski at gmail.com> wrote:
>> >>
>> >> On Fri, Nov 24, 2017 at 4:00 PM, Piotr Kliczewski
>> >> <piotr.kliczewski at gmail.com> wrote:
>> >> > On Fri, Nov 24, 2017 at 2:20 PM, Nir Soffer <nsoffer at redhat.com>
>> >> > wrote:
>> >> >> Adding devel at ovirt, the proper mailing list
>> >> >>
>> >> >> בתאריך יום ו׳, 24 בנוב׳ 2017, 7:42, מאת Germano Veit Michel
>> >> >> ‏<germano at redhat.com>:
>> >> >>>
>> >> >>> 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
>> >> >> Devel at ovirt.org
>> >> >> http://lists.ovirt.org/mailman/listinfo/devel
>
>
>
>
> --
> Germano Veit Michel <germano at redhat.com>
> Senior Software Maintenance Engineer, Virtualization, Red Hat