New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 623999 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
User never visited
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

cros deploy fails when using progress bar operation

Project Member Reported by cburden@google.com, Jun 28 2016

Issue description

cros deploy crashes when output is piped and log-level is not specified:

$ cros deploy $remote cryptohome | cat
09:30:24: NOTICE: Cleaning outdated binary packages from /build/daisy
09:30:26: NOTICE: These are the packages to emerge:
09:30:26: NOTICE: * 1) chromeos-base/cryptohome-0.0.1-r1220
09:30:26: ERROR: cros deploy failed before completing.
09:30:26: ERROR: _EmergePackages() got an unexpected keyword argument 'log_level'

The actual use case here is running cros deploy through a bash script executed from utils.command_executer. Setting --log-level=notice appears to fix it.
 

Comment 1 by cburden@google.com, Jun 28 2016

Actually, --log-level=notice does not work, it gives the same error as not providing --log-level at all. The best work around I can find is giving --log-level=info

Comment 2 by cmt...@chromium.org, Jun 28 2016

Labels: Build-Tools

Comment 3 by vapier@chromium.org, Jun 28 2016

Summary: cros deploy fails when using progress bar operation (was: cros deploy fails with piped output and unset --log-level)
while we should fix this python error, it won't really help you -- the code is prompting you for input and you haven't provided any

looks like it's:
cli/deploy.py:
      if emerge:
        func = functools.partial(_EmergePackages, pkgs, device, strip,
                                 sysroot, root, emerge_args)
      else:
        func = functools.partial(_UnmergePackages, pkgs, device, root)

      if command.UseProgressBar():
        op = BrilloDeployOperation(len(pkgs), emerge)
        op.Run(func, log_level=logging.DEBUG)

that code will turn around and pass |log_level| to |func|, but neither of those funcs accept |log_level|.  should just drop it i think.
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 29 2016

Labels: Hotlist-Google
Labels: -Pri-3 Pri-2
raising priority a little. I was just hit by this. It is annoying not to be able to do this..
Status: Started (was: Untriaged)
Assuming this is available, Matteo got a fix for it:
https://chromium-review.googlesource.com/c/445787/

If it is ok, I will mark it as started (and next assign to someone at our team).
Owner: vasileio...@arm.com
Assigning it to one of our team mates in UK.
Cc: cavalcantii@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/e30397d83d7bf84734c00e9776ef703f75d44d23

commit e30397d83d7bf84734c00e9776ef703f75d44d23
Author: Matteo Franchin <matteo.franchin@arm.com>
Date: Tue Mar 07 14:16:14 2017

Always consume log_level arg in ProgressBarOperation.Run

Fix inconsistent handling of the log_level keyword argument in
ProgressBarOperation.Run(). This method was indeed passing the log_level
keyword argument to its callback in certain cases, but not in others.
In particular, when running cros deploy with redirected stdout (isatty()
returning False) Run() passed the log_level argument to its callback.
This caused failures to run cros deploy when redirecting its output to a
file, for example.

This commit fixes the issue by changing the Run() method so that it
always consumes the log_level argument and uses it to set the log level
under which the callback function is ran.

BUG= chromium:623999 
TEST=lib/operation_unittest cli/deploy_unittest

Change-Id: I846d3868d3274d01d017a4b80b84eb91345586b0
Reviewed-on: https://chromium-review.googlesource.com/445787
Commit-Ready: Matteo Franchin <matteo.franchin@arm.com>
Tested-by: Matteo Franchin <matteo.franchin@arm.com>
Reviewed-by: Chris Sosa <sosa@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/e30397d83d7bf84734c00e9776ef703f75d44d23/lib/operation.py

Status: Fixed (was: Started)
As the patch landed, I'm marking the issue as Fixed.

Comment 11 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61
Status: Verified (was: Fixed)
Closing. Please reopen it if its not fixed. Thanks!

Sign in to add a comment