Closed Bug 1307482 Opened 8 years ago Closed 8 years ago

scp files concurrently

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox52 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: gps, Assigned: gps)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Bug 1307482 - Refactor remote path logic into function;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1307482 - Avoid excessive scp calls to make directories;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1307482 - Log uploading when it actually happens;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1307482 - Upload files concurrently;
58 bytes, text/x-review-board-request
ted
: review+
Details 
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.

Attachment

General

Creator:
Created:
Updated:
Size: