Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert nightly upload script to Python #16565

Merged

Conversation

@aneeshusa
Copy link
Member

aneeshusa commented Apr 21, 2017

Now that MinGW and MSYS have been removed from the Windows builders,
bash is not available to run the previous upload_nightlies.sh script.
Convert the script to Python 2 for cross-platform support.
Additionally, switch to the boto3 library for easy uploading
without needing to install s3cmd,
and move the code into mach for easy boto3 installation
as the new ./mach upload-nightly command.

Also, hard-code the paths to the packages instead of using
globs to look for them, as the paths are static.
(The paths used to contain timestamps,
but we now insert timestamps when uploading to S3
to improve reproducibility.)


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #16560 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because Buildbot will test them.

This change is Reviewable

@aneeshusa
Copy link
Member Author

aneeshusa commented Apr 21, 2017

@highfive highfive assigned larsbergstrom and unassigned KiChjang Apr 21, 2017
@aneeshusa aneeshusa force-pushed the aneeshusa:move-upload-nightly-script-to-python branch from c775e3d to c102ba6 Apr 21, 2017
package_upload_path,
])
subprocess.check_call([
'echo',

This comment has been minimized.

@metajack

metajack Apr 27, 2017

Contributor

Are these calls supposed to run s3cmd not echo?

with tempfile.TemporaryDirectory(prefix='homebrew-servo') as tmp_dir:
def call_git(cmd, **kwargs):
subprocess.check_call(
['echo', 'git', '-C', tmp_dir] + cmd,

This comment has been minimized.

@metajack

metajack Apr 27, 2017

Contributor

Same here.

@metajack metajack assigned metajack and unassigned larsbergstrom Apr 27, 2017
@aneeshusa
Copy link
Member Author

aneeshusa commented Apr 27, 2017

s3cmd apparently is not (well) supported on Windows, so I'll rework this to use a different tool.

@aneeshusa aneeshusa changed the title Convert nightly upload script to Python 3 WIP: Convert nightly upload script to Python Apr 27, 2017
@metajack
Copy link
Contributor

metajack commented Apr 27, 2017

Can we just use the aws cli tool directly?

@metajack
Copy link
Contributor

metajack commented May 8, 2017

@aneeshusa I'm happy to drive this across the line if you are busy, btw.

@aneeshusa
Copy link
Member Author

aneeshusa commented May 10, 2017

@metajack I would appreciate that! Here's where I was thinking of taking this:

  • Avoid installing anything globally/pushing anything via saltfs.
  • Port this to Python 2 and move this into mach.
  • Use boto3 as a python library instead of s3cmd via the command line. Install boto3 as part of the mach Python requirements.

This should hopefully sidestep any Windows compatibility issues.

@aneeshusa aneeshusa force-pushed the aneeshusa:move-upload-nightly-script-to-python branch from c102ba6 to 1b21126 May 18, 2017
@aneeshusa
Copy link
Member Author

aneeshusa commented May 18, 2017

I've done the steps from my last comment and this is ready for review. I did some light smoke testing, but haven't tested actually uploading to S3 or the update_brew code.

@atouchet
Copy link
Contributor

atouchet commented May 19, 2017

Is this still a WIP or can that tag be removed?

@aneeshusa aneeshusa changed the title WIP: Convert nightly upload script to Python Convert nightly upload script to Python May 19, 2017
@larsbergstrom
Copy link
Contributor

larsbergstrom commented May 20, 2017

@metajack @edunham

It looks to me like this is good to go. Should we r+ and then check to see what happens on the nightly builds?

cc @jonathandturner

Now that MinGW and MSYS have been removed from the Windows builders,
bash is not available to run the previous upload_nightlies.sh script.
Convert the script to Python 2 for cross-platform support.
Additionally, switch to the `boto3` library for easy uploading
without needing to install `s3cmd`,
and move the code into mach for easy `boto3` installation
as the new `./mach upload-nightly` command.

Also, hard-code the paths to the packages instead of using
globs to look for them, as the paths are static.
(The paths used to contain timestamps,
but we now insert timestamps when uploading to S3
to improve reproducibility.)
@aneeshusa aneeshusa force-pushed the aneeshusa:move-upload-nightly-script-to-python branch from 1b21126 to 043f78d May 20, 2017
@metajack
Copy link
Contributor

metajack commented May 22, 2017

@bors-servo
Copy link
Contributor

bors-servo commented May 22, 2017

📌 Commit 043f78d has been approved by metajack

@metajack
Copy link
Contributor

metajack commented May 22, 2017

Great work @aneeshusa. This looks like a huge improvement.

@bors-servo
Copy link
Contributor

bors-servo commented May 22, 2017

Testing commit 043f78d with merge 9468fae...

bors-servo added a commit that referenced this pull request May 22, 2017
…, r=metajack

Convert nightly upload script to Python

Now that MinGW and MSYS have been removed from the Windows builders,
bash is not available to run the previous upload_nightlies.sh script.
Convert the script to Python 2 for cross-platform support.
Additionally, switch to the `boto3` library for easy uploading
without needing to install `s3cmd`,
and move the code into mach for easy `boto3` installation
as the new `./mach upload-nightly` command.

Also, hard-code the paths to the packages instead of using
globs to look for them, as the paths are static.
(The paths used to contain timestamps,
but we now insert timestamps when uploading to S3
to improve reproducibility.)

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #16560 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because Buildbot will test them.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16565)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 23, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: metajack
Pushing 9468fae to master...

@bors-servo bors-servo merged commit 043f78d into servo:master May 23, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
bors-servo added a commit to servo/saltfs that referenced this pull request May 24, 2017
…y, r=jdm

Provide secrets for `./mach upload-nightly`

Also remove s3cmd related code,
as we now use boto3 for uploads.

Goes along with servo/servo#16565.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/675)
<!-- Reviewable:end -->
@aneeshusa aneeshusa mentioned this pull request May 29, 2017
2 of 5 tasks complete
bors-servo added a commit that referenced this pull request May 30, 2017
… r=larsbergstrom

Fix `./mach upload-nightly macbrew`

<!-- Please describe your changes on the following line: -->
Fix a few issues with the Homebrew repository updating.
Follow-up to #16565; should finish off #17045.

Lightly smoke-tested locally this time.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they will be tested on the builders

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17085)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants
You can’t perform that action at this time.