download_sdk_extras: Catch the correct error.

download_sdk_extras: Catch the correct error.

Catch the error raised by 'check_call' rather than the error emitted by
the called process.

BUG=chromium:460693, chromium:459681
TEST=local
  - Ran without fix, setting CHROME_HEADLESS=1. Script failed with the exception
    in the bug.
  - Ran with fix, 'gsutil' failed with AccessDeniedException output
    and the calling script successfully emitted a warning message.

R=hinoka@chromium.org, navabi@chromium.org

Committed: https://crrev.com/63ab4fead51b043cb1f21ae2b31f9e593cbdc3ca
Cr-Commit-Position: refs/heads/master@{#317608}

[dnj, 2015-02-23 02:12:09]

The script is currently not executable on a bot. This is breaking CrOS PFQ and
probably other builds. This should fix the problem; PTAL!

[navabi, 2015-02-23 17:40:48]

navabi@google.com changed reviewers:
+ navabi@google.com

[navabi, 2015-02-23 17:40:49]

https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py
File build/download_sdk_extras.py (right):

https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py...
build/download_sdk_extras.py:63: print ('WARNING: Failed to download SDK
packages. If this bot compiles '
We still need to catch the AccessDeniedExceptions for cases where the call works
but the bot does not have gstuil credentials.

[dnj, 2015-02-23 17:43:17]

https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py
File build/download_sdk_extras.py (right):

https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py...
build/download_sdk_extras.py:63: print ('WARNING: Failed to download SDK
packages. If this bot compiles '
On 2015/02/23 17:40:49, navabi wrote:
> We still need to catch the AccessDeniedExceptions for cases where the call
works
> but the bot does not have gstuil credentials.

We can't catch AccessDeniedException. gsutil, which emits the exception, is
executing in a subprocess, so the only feedback that this script gets is the
subprocess return code, not the actual exception.

This construction catches that and prints the warning. It (something) looks
like:

AccessDeniedException: 403 << output from gsutil, causing it to return non-zero.
WARNING: Failed to download SDK packages... << output from this 'print'.

[navabi, 2015-02-23 17:46:54]

lgtm

[navabi1, 2015-02-23 17:47:08]

lgtm

[gwendal, 2015-02-23 17:48:36]

The CQ bit was checked by gwendal@chromium.org

[gwendal, 2015-02-23 17:48:38]

The CQ bit was unchecked by gwendal@chromium.org

[dnj, 2015-02-23 17:48:40]

dnj@chromium.org changed reviewers:
- gwendal@chromium.org

[dnj, 2015-02-23 17:49:11]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2015-02-23 17:49:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/949813002/1

[gwendal, 2015-02-23 18:14:46]

gwendal@chromium.org changed reviewers:
+ cjhopman@chromium.org, jochen@chromium.org, scottmg@chromium.org,
thakis@chromium.org

[dnj, 2015-02-23 18:16:28]

+owners, can someone LGTM please :)

[cjhopman, 2015-02-23 18:18:53]

lgtm

[scottmg, 2015-02-23 18:19:44]

lgtm

https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py
File build/download_sdk_extras.py (right):

https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py...
build/download_sdk_extras.py:63: print ('WARNING: Failed to download SDK
packages. If this bot compiles '
On 2015/02/23 17:43:17, dnj wrote:
> On 2015/02/23 17:40:49, navabi wrote:
> > We still need to catch the AccessDeniedExceptions for cases where the call
> works
> > but the bot does not have gstuil credentials.
> 
> We can't catch AccessDeniedException. gsutil, which emits the exception, is
> executing in a subprocess, so the only feedback that this script gets is the
> subprocess return code, not the actual exception.
> 
> This construction catches that and prints the warning. It (something) looks
> like:
> 
> AccessDeniedException: 403 << output from gsutil, causing it to return
non-zero.
> WARNING: Failed to download SDK packages... << output from this 'print'.

Is is useful to print the content of the exception here? (e.g. for the command
line)

[dnj, 2015-02-23 18:22:03]

On 2015/02/23 18:19:44, scottmg wrote:
> lgtm
> 
> https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py
> File build/download_sdk_extras.py (right):
> 
>
https://codereview.chromium.org/949813002/diff/1/build/download_sdk_extras.py...
> build/download_sdk_extras.py:63: print ('WARNING: Failed to download SDK
> packages. If this bot compiles '
> On 2015/02/23 17:43:17, dnj wrote:
> > On 2015/02/23 17:40:49, navabi wrote:
> > > We still need to catch the AccessDeniedExceptions for cases where the call
> > works
> > > but the bot does not have gstuil credentials.
> > 
> > We can't catch AccessDeniedException. gsutil, which emits the exception, is
> > executing in a subprocess, so the only feedback that this script gets is the
> > subprocess return code, not the actual exception.
> > 
> > This construction catches that and prints the warning. It (something) looks
> > like:
> > 
> > AccessDeniedException: 403 << output from gsutil, causing it to return
> non-zero.
> > WARNING: Failed to download SDK packages... << output from this 'print'.
> 
> Is is useful to print the content of the exception here? (e.g. for the command
> line)

At this point it'd just be a return code. Since the STDOUT/STDERR from the
subprocess is already being dumped and it shows the error message, I'm thinking
not.

[commit-bot: I haz the power, 2015-02-23 18:22:04]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-23 18:22:51]

Patchset 1 (id:??) landed as
https://crrev.com/63ab4fead51b043cb1f21ae2b31f9e593cbdc3ca
Cr-Commit-Position: refs/heads/master@{#317608}