Closed Bug 1247490 Opened 4 years ago Closed 4 years ago

upload.py hides ssh command output if the command fails

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(1 file)

Since upload.py now uses subprocess.check_output when running ssh commands, we don't see the output if the command fails. We should catch the CalledProcessError exception and display the output in this case in order to see the actual error message.
See Also: → 1217987
Comment on attachment 8718402 [details] [diff] [review]
0001-Bug-1247490-display-ssh-output-if-upload-command-fai.patch

Review of attachment 8718402 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/upload.py
@@ +37,5 @@
>  import errno
>  import hashlib
>  import shutil
>  from optparse import OptionParser
> +from subprocess import check_call, check_output, STDOUT, CalledProcessError

Might be nice to make this multi-line:
from subprocess import (
...
)

@@ +96,5 @@
> +        try:
> +            output = f(cmdline, stderr=STDOUT).strip()
> +        except CalledProcessError as e:
> +            print "failed ssh command output:"
> +            print e.output

Wrap the output in delimiting lines? like '=' * 20 or something.
Attachment #8718402 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> ::: build/upload.py
> @@ +37,5 @@
> >  import errno
> >  import hashlib
> >  import shutil
> >  from optparse import OptionParser
> > +from subprocess import check_call, check_output, STDOUT, CalledProcessError
> 
> Might be nice to make this multi-line:
> from subprocess import (
> ...
> )

Fixed.

> 
> @@ +96,5 @@
> > +        try:
> > +            output = f(cmdline, stderr=STDOUT).strip()
> > +        except CalledProcessError as e:
> > +            print "failed ssh command output:"
> > +            print e.output
> 
> Wrap the output in delimiting lines? like '=' * 20 or something.

Good idea, fixed as well.
https://hg.mozilla.org/mozilla-central/rev/67f91747f48e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.