> -----Original Message-----
> From: Ben Nemec [mailto:openst...@nemebean.com]
> Sent: 26 November 2014 17:03
> To: OpenStack Development Mailing List (not for usage questions)
> Subject: Re: [openstack-dev] [diskimage-builder] Tracing levels for
> scripts (119023)
> On 11/25/2014 10:58 PM, Ian Wienand wrote:
> > Hi,
> >
> > My change [1] to enable a consistent tracing mechanism for the many
> > scripts diskimage-builder runs during its build seems to have hit a
> > stalemate.
> >
> > I hope we can agree that the current situation is not good.  When
> > trying to develop with diskimage-builder, I find myself constantly
> > going and fiddling with "set -x" in various scripts, requiring me
> > re-running things needlessly as I try and trace what's happening.
> > Conversley some scripts set -x all the time and give output when you
> > don't want it.
> >
> > Now nodepool is using d-i-b more, it would be even nicer to have
> > consistency in the tracing so relevant info is captured in the image
> > build logs.
> >
> > The crux of the issue seems to be some disagreement between reviewers
> > over having a single "trace everything" flag or a more fine-grained
> > approach, as currently implemented after it was asked for in reviews.
> >
> > I must be honest, I feel a bit silly calling out essentially a
> > four-line patch here.
> My objections are documented in the review, but basically boil down to
> the fact that it's not a four line patch, it's a 500+ line patch that
> does essentially the same thing as:
> set +e
> set -x
> export SHELLOPTS

I don't think this is true, as there are many more things in SHELLOPTS than 
just xtrace.  I think it is wrong to call the two approaches equivalent.

> in disk-image-create.  You do lose set -e in disk-image-create itself on
> debug runs because that's not something we can safely propagate,
> although we could work around that by unsetting it before calling hooks.
>  FWIW I've used this method locally and it worked fine.

So this does say that your alternative implementation has a difference from the 
proposed one.  And that the difference has a negative impact.

> The only drawback is it doesn't allow the granularity of an if block in
> every script, but I don't personally see that as a particularly useful
> feature anyway.  I would like to hear from someone who requested that
> functionality as to what their use case is and how they would define the
> different debug levels before we merge an intrusive patch that would
> need to be added to every single new script in dib or tripleo going
> forward.

So currently we have boilerplate to be added to all new elements, and that 
boilerplate is:

    set -eux
    set -o pipefail

This patch would change that boilerplate to:

    if [ ${DIB_DEBUG_TRACE:-0} -gt 0 ]; then
        set -x
    set -eu
    set -o pipefail

So it's adding 3 lines.  It doesn't seem onerous, especially as most people 
creating a new element will either copy an existing one or copy/paste the 
header anyway.

I think that giving control over what is effectively debug or non-debug output 
is a desirable feature.

We have a patch that implements that desirable feature.

I don't see a compelling technical reason to reject that patch.

> -Ben
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Jon-Paul Sullivan ☺ Cloud Services - @hpcloud

Postal Address: Hewlett-Packard Galway Limited, Ballybrit Business Park, Galway.
Registered Office: Hewlett-Packard Galway Limited, 63-74 Sir John Rogerson's 
Quay, Dublin 2. 
Registered Number: 361933
The contents of this message and any attachments to it are confidential and may 
be legally privileged. If you have received this message in error you should 
delete it from your system immediately and advise the sender.

To any recipient of this message within HP, unless otherwise stated, you should 
consider this message and attachments as "HP CONFIDENTIAL".
OpenStack-dev mailing list

Reply via email to