Open Stack

Hi!

On Fri, Jun 13, 2014 at 9:30 AM, Day, Phil <philip.day at hp.com> wrote:

>  Hi Folks,
>
>
>
> A recent change introduced a unit test to “warn/notify developers” when
> they make a change which will break the out of tree Ironic virt driver:
> https://review.openstack.org/#/c/98201
>
>
>
> Ok – so my change (https://review.openstack.org/#/c/68942) broke it as it
> adds some extra parameters to the virt drive power_off() method – and so I
> now feel suitable warned and notified – but am not really clear what I’m
> meant to do next.
>
>
>
> So far I’ve:
>
> -          Modified the unit test in my Nova patch so it now works
>
> -          Submitted an Ironic patch to add the extra parameters (
> https://review.openstack.org/#/c/99932/)
>
>
>
> As far as I can see there’s no way to create a direct dependency from the
> Ironic change to my patch – so I guess its down to the Ironic folks to wait
> and accept it in the correct sequence ?
>

Thanks for bringing up this question.

98201 was added at the suggestion of Sean Dague during a conversation
in #openstack-infra to help prevent terrible breakages that affect the gate.
What wasn't discussed, however, is how we should coordinate these changes
going forward.

As for your change, I think what you've done is exactly what we had hoped
would
be done. In your particular case, I don't see any need for Nova dev's to
not go ahead
and approve 68942 *before* 99932 since defaults are added to the arguments.
The
question is, how do we coordinate such changes if a change DOES actually
break
ironic?

One suggestion is that if test_ironic_api_contracts.py
<https://review.openstack.org/#/c/68942/15/nova/tests/virt/test_ironic_api_contracts.py>
is
ever changed, Nova require
the Ironic PTL (or a core dev) to vote before approving. That seems
sensible to me.
There might be an easier way of coordinating that I'm overlooking, though.

-Dave
--
David Shrewsbury (Shrews)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140613/0e15326a/attachment.html>