Excerpts from James Slagle's message of 2014-11-28 11:27:20 -0800: > On Thu, Nov 27, 2014 at 1:29 PM, Sullivan, Jon Paul > <jonpaul.sulli...@hp.com> wrote: > >> -----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 > > fi > > 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. > > I don't think it's debug vs non-debug. I think script writers that > have explicitly used set -x previously have then operated under the > assumption that they don't need to add any useful logging since it's > running -x. In that case, this patch is actually harmful. >
I believe James has hit the nail squarely on the head with the paragraph above. I propose a way forward for this: 1) Conform all o-r-c scripts to the logging standards we have in OpenStack, or write new standards for diskimage-builder and conform them to those standards. Abolish non-conditional xtrace in any script conforming to the standards. 2) Once that is done, implement optional -x. I rather prefer the explicit conditional set -x implementation over SHELLOPTS. As somebody else pointed out, it feels like asking for unintended side-effects. But the "how" is far less important than the "what" in this case, which step 1 will better define. Anyone else have a better plan? _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev