Status

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks 1 bug)

unspecified
mozilla52
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(4 attachments)

Currently, upload.py scp's files 1 at a time. We can perform multiple scp operations concurrently to make upload complete faster.
Comment on attachment 8797650 [details]
Bug 1307482 - Log uploading when it actually happens;

https://reviewboard.mozilla.org/r/83308/#review81802

::: build/upload.py:118
(Diff revision 1)
> -def DoSCPFile(file, remote_path, user, host, port=None, ssh_key=None):
> +def DoSCPFile(file, remote_path, user, host, port=None, ssh_key=None,
> +              log=False):
>      """Upload file to user@host:remote_path using scp. Optionally use
>      port and ssh_key, if provided."""
> +    if log:
> +        print('Uploading %s' % file)

Drive-by nit: I'm not a fan of print_function in files where `print "foo"` is valid (and used a lot already)
Comment on attachment 8797648 [details]
Bug 1307482 - Refactor remote path logic into function;

https://reviewboard.mozilla.org/r/83304/#review85152
Attachment #8797648 - Flags: review?(ted) → review+
Comment on attachment 8797649 [details]
Bug 1307482 - Avoid excessive scp calls to make directories;

https://reviewboard.mozilla.org/r/83306/#review85154

::: build/upload.py:245
(Diff revision 1)
>                  raise IOError("File not found: %s" % file)
> -            # first ensure that path exists remotely
> +
> +            remote_paths.add(get_remote_path(file))
> +
> +        # If we wanted to, we could reduce the remote paths if they are a parent
> +        # of any entry.

This seems like it might actually be worth doing, since some of the paths are almost certainly going to be subdirs of others, so we don't have to run mkdir for the parent dirs if we're doing `mkdir -p` for the children.
Attachment #8797649 - Flags: review?(ted) → review+
Comment on attachment 8797650 [details]
Bug 1307482 - Log uploading when it actually happens;

https://reviewboard.mozilla.org/r/83308/#review85166

::: build/upload.py:118
(Diff revision 1)
> -def DoSCPFile(file, remote_path, user, host, port=None, ssh_key=None):
> +def DoSCPFile(file, remote_path, user, host, port=None, ssh_key=None,
> +              log=False):
>      """Upload file to user@host:remote_path using scp. Optionally use
>      port and ssh_key, if provided."""
> +    if log:
> +        print('Uploading %s' % file)

This file doesn't have `print_function` imported, so you either need to use a statement or do that and fix the other uses. (Or just switch the whole thing to logging while you're at it or whatever you feel is best.)
Attachment #8797650 - Flags: review?(ted) → review+
Comment on attachment 8797651 [details]
Bug 1307482 - Upload files concurrently;

https://reviewboard.mozilla.org/r/83310/#review85170

::: build/upload.py:266
(Diff revision 2)
> -            remote_files.append(remote_path + '/' + os.path.basename(file))
> +                remote_files.append(remote_path + '/' + os.path.basename(file))
> +
> +            # We need to call result() on the future otherwise exceptions could
> +            # get swallowed.
> +            for f in futures.as_completed(fs):
> +                f.result()

Would you want to log completion here?
Attachment #8797651 - Flags: review?(ted) → review+
Comment on attachment 8797650 [details]
Bug 1307482 - Log uploading when it actually happens;

https://reviewboard.mozilla.org/r/83308/#review85166

> This file doesn't have `print_function` imported, so you either need to use a statement or do that and fix the other uses. (Or just switch the whole thing to logging while you're at it or whatever you feel is best.)

I'm just going to use the legacy print statement to match the rest of the file.
Comment on attachment 8797651 [details]
Bug 1307482 - Upload files concurrently;

https://reviewboard.mozilla.org/r/83310/#review85170

> Would you want to log completion here?

Possibly. Could be done as a follow-up: this series maintains status quo w.r.t. logging.
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7319e764beb9
Refactor remote path logic into function; r=ted
https://hg.mozilla.org/integration/autoland/rev/8764e3613653
Avoid excessive scp calls to make directories; r=ted
https://hg.mozilla.org/integration/autoland/rev/1bcff6b5f303
Log uploading when it actually happens; r=ted
https://hg.mozilla.org/integration/autoland/rev/882da7566779
Upload files concurrently; r=ted
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.