Skip to content

Bazel mirrors#7385

Merged
simon-mo merged 15 commits intoray-project:masterfrom
mehrdadn:bazel-mirrors
Mar 1, 2020
Merged

Bazel mirrors#7385
simon-mo merged 15 commits intoray-project:masterfrom
mehrdadn:bazel-mirrors

Conversation

@mehrdadn
Copy link
Copy Markdown
Contributor

@mehrdadn mehrdadn commented Mar 1, 2020

Why are these changes needed?

  • dl.bintray.com keeps returning forbidden, so we want to be able to use a mirror

  • Improve build performance

  • Make build rules clearer and more succinct

Checks

@AmplabJenkins
Copy link
Copy Markdown

Can one of the admins verify this patch?

@simon-mo
Copy link
Copy Markdown
Contributor

simon-mo commented Mar 1, 2020

It seems most of the http_archive are really just github archive, can you point me to where did we solve the bintray issue?

@simon-mo simon-mo self-requested a review March 1, 2020 05:57
@mehrdadn
Copy link
Copy Markdown
Contributor Author

mehrdadn commented Mar 1, 2020

It's the mirror here :) f9b23a9#diff-5d1661b10448a2754e2c6c9ecd896d8bR34

@simon-mo
Copy link
Copy Markdown
Contributor

simon-mo commented Mar 1, 2020

any official documentation on the existence of mriror.bazel.build? who manages it?

@mehrdadn
Copy link
Copy Markdown
Contributor Author

mehrdadn commented Mar 1, 2020

I don't think it's officially documented, but it's used by Bazel itself for lots of downloads, as well as repos like rules_boost (which we've used). It's managed by the Bazel folks. I think it's pretty safe :-)

Copy link
Copy Markdown
Contributor

@simon-mo simon-mo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, had few questions


env:
LLVM_VERSION_WINDOWS: 9.0.0
GITHUB: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh man I was looking for these, couldn't find them. Thanks!

strip_prefix=True, url=None, path=None, **kwargs):
def urlsplit(url):
""" Splits a URL like "https://example.com/a/b?c=d&e#f" into a tuple:
("https", ["example", "com"], ["a", "b"], ["c=d", "e"], "f")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can if you want, but I thought it's nicer to keep them in order and somewhat similar to Python's own urlsplit (which returns a tuple).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but python returns named tuple which are a bit more readable when retrieving the result.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha yeah true, ok sure I'll change it.

def auto_http_archive(*, name=None, url=None, urls=True,
build_file=None, build_file_content=None,
strip_prefix=True, **kwargs):
""" Intelligently choose mirrors based on the given URL for the download.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one sentence about the heuristic we are using here? (when to use bazel mirror and when to use github directly?)

Copy link
Copy Markdown
Contributor Author

@mehrdadn mehrdadn Mar 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the heuristic is we prioritize mirror.bazel.build URLs below GitHub URLs, but above everything else.

(The line you're looking for is prefer_url_over_mirrors = is_github.)

@AmplabJenkins
Copy link
Copy Markdown

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/22585/
Test FAILed.

@AmplabJenkins
Copy link
Copy Markdown

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/22586/
Test FAILed.

@AmplabJenkins
Copy link
Copy Markdown

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/22587/
Test PASSed.

@simon-mo simon-mo merged commit 44aded5 into ray-project:master Mar 1, 2020
@mehrdadn mehrdadn deleted the bazel-mirrors branch March 1, 2020 22:05
stephanie-wang added a commit to stephanie-wang/ray that referenced this pull request Mar 3, 2020
stephanie-wang added a commit to stephanie-wang/ray that referenced this pull request Mar 3, 2020
stephanie-wang added a commit that referenced this pull request Mar 14, 2020
* Add a test for LRU fallback

* Update error message

* Upgrade arrow to master

* Integrate with arrow

* Revert "Bazel mirrors (#7385)"

This reverts commit 44aded5.

* Don't LRU evict

* Revert "Revert "Bazel mirrors (#7385)""

This reverts commit b6359fe.

* Add lru_evict flag

* fix internal config

* Fix

* upgrade arrow

* debug

* Set free period in config for lru_evict, override max retries to fix
test

* Fix test?

* fix test

* Revert "debug"

This reverts commit 98f01c6.

* fix exception str

* Fix ref count test

* Shorten travis test?
ffbin pushed a commit to antgroup/ant-ray that referenced this pull request Mar 20, 2020
* Switch to mirrors.bazel.build where possible

* Switch from .zip to .tar.gz for smaller downloads (it's also the default download on UNIX)

* Use direct GitHub URLs in Bazel files for clarity

* Don't pass patches to local_repository

* Remove github_repository()

* Switch to GitHub actions/checkout@v2 which is faster

* Use faster extraction method for LLVm on Windows

* Move LLVM_VERSION_WINDOWS to the shell script since it's not a CI-specific value

* Change GITHUB_TOKEN to GITHUB

* Don't show timestamps for GitHub Actions

* Factor out some options from GitHub Actions

* Tell Bazel to stay on the same volume in GitHun Actions

* Display progress output when downloading toolchains

Co-authored-by: GitHub Web Flow <noreply@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants