Skip to main content

[pkg-discuss] Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

  • From: Xiaobo Shen < >
  • To: " " < >
  • Subject: [pkg-discuss] Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command
  • Date: Wed, 18 Dec 2013 11:43:08 -0800

This one is almost on its way to put back. Could somebody send more suggestions? Thanks.

xiaobo


-------- Original Message --------
Subject: [pkg-discuss] Re: Review request -> 17734557 Bad formatting of error text from pkg update command
Date:   Fri, 15 Nov 2013 11:20:23 -0800
From:   Xiaobo Shen 
< >
Reply-To:       

To:     




On 11/13/13 07:10 AM, Erik Trauschke wrote:


On 11/12/13 04:31 PM, Xiaobo Shen wrote:
On 11/12/13 01:22 PM, Erik Trauschke wrote:


On 11/12/13 12:48 PM, Tim Foster wrote:
On 11/13/13 09:10 AM, Xiaobo Shen wrote:
On 11/11/13 01:40 PM, Tim Foster wrote:
That's the easiest way to fix the problem, but it's not the best way
to do it.

I found we might call flush() in exception handling. It works and the
output is the same as above.
Please see:
https://ips.java.net/webrev/xiaoshen/pkgbadformat_17734557_2/

That's much nicer - thanks. I'm happy with the changes as you have
them.

As Bart is fond of saying, "all bugs have friends" - if you have time,
it could be interesting to have a quick scan through the rest of the
code in the gate to see if there's other places where we might run
into
the same issue with clients using the ProgressTracker.  You don't need
to fix them in this wad, but filing bugs against them would be
helpful.

- if you don't have time, that's ok.

I'm wondering if we should put the flush into src/client.py:error().
The imageplan shouldn't be bothered with correct print-outs of error
messages, this should be the job of the CLI code.
I found that the msg "The running system has not been modified...." is
from restore_image call in client/bootenv.py. This msg will be printed
before handling exception msgs in client.py. For instance,
If we press ctr_c, the latest place we need a "\n" is before
be.restore_image() at line 2682 in api.py.
I can add the flush() call at the beginning of this exception handling
and a couple of other ones. Let me
know if it is applicable. Otherwise we need to figure out some other
ways.

Ah, yes. This happens because it uses logger directly instead of
ProgressTracker. This looks like a good follow-on bug to fix.
However, seeing that all of bootenv.py uses logging instead of
exceptions and doesn't use ProgressTracker at all this could be a bit
more involved.

I think in this case we'd have to print a "\n" before the message
since we have no access to the ProgressTracker. I'm also wondering if
we would run into the same issue with all the other logger.error()
calls in this class.

I'm not too familiar with the ProgressTracker but would it be an issue
to run a flush every time we print messages with error()?

After flush(), The ProgressTracker will set the __need_nl in printengine
into false. So I did not see a problem, but I can test it for sure.

Cool, thanks

Erik
I did some change about the bootenv.py to allow it using
progress_tracker flush() before
the log msgs are printed out. Let me know what you guys think. Thanks.

new link:
https://ips.java.net/webrev/xiaoshen/pkgbadformat_17734557_2/

Xiaobo





[pkg-discuss] Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

Xiaobo Shen 12/18/2013

[pkg-discuss] Re: Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

Erik Trauschke 12/18/2013

[pkg-discuss] Re: Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

Xiaobo Shen 12/19/2013

[pkg-discuss] Re: Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

Erik Trauschke 12/20/2013

[pkg-discuss] Re: Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

Xiaobo Shen 12/20/2013

[pkg-discuss] Re: Fwd: Re: Review request -> 17734557 Bad formatting of error text from pkg update command

Erik Trauschke 12/20/2013
 
 
Close
loading
Please Confirm
Close