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