Skip to content

If cache.julialang.org is down, try to download Busybox from the upstream URL#47015

Merged
DilumAluthge merged 1 commit intomasterfrom
dpa/busybox-cache-julialang-org
Oct 4, 2022
Merged

If cache.julialang.org is down, try to download Busybox from the upstream URL#47015
DilumAluthge merged 1 commit intomasterfrom
dpa/busybox-cache-julialang-org

Conversation

@DilumAluthge
Copy link
Copy Markdown
Member

@DilumAluthge DilumAluthge commented Oct 3, 2022 •
edited
Loading

  1. If cache.julialang.org is down, try to download Busybox from the upstream URL
  2. Wrap the entire download process inside a bounded retry loop

Co-authored-by: Lilith Hafner [email protected]

@DilumAluthge DilumAluthge added system:windows Affects only Windows test This change adds or pertains to unit tests labels Oct 3, 2022
@DilumAluthge DilumAluthge mentioned this pull request Oct 3, 2022
Closed
@DilumAluthge DilumAluthge force-pushed the dpa/busybox-cache-julialang-org branch from 6552cd2 to 6781838 Compare October 3, 2022 01:56
@DilumAluthge
Copy link
Copy Markdown
Member Author

DilumAluthge commented Oct 3, 2022

Does anyone know how to make the commit coauthorship show up correctly?

LilithHafner
LilithHafner reviewed Oct 3, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

Even with a backup URL and retries, there is still a chance that the download fails. Given that we already have a case for busybox not being available, I think it makes sense to put the download in the try block as well as the integrity test.

@DilumAluthge
Copy link
Copy Markdown
Member Author

DilumAluthge commented Oct 3, 2022 •
edited
Loading

Even with a backup URL and retries, there is still a chance that the download fails.

Yes, but we want that to lead to a CI failure, so that we notice. If downloads from the upstream URL are failing frequently enough for us to notice, then we likely need to identify a different mirror for this download.

@LilithHafner
Copy link
Copy Markdown
Member

LilithHafner commented Oct 3, 2022

Is the same true for the case where success(`$busybox`) fails?

@DilumAluthge
Copy link
Copy Markdown
Member Author

DilumAluthge commented Oct 3, 2022

Is the same true for the case where success(`$busybox`) fails?

Yeah, I was just looking at that. I was thinking that for local use, maybe we allow that to fail, but on CI, we throw an error if it fails.

So e.g. we check haskey(ENV, "CI") or something like that.

@DilumAluthge DilumAluthge force-pushed the dpa/busybox-cache-julialang-org branch from 6781838 to 16e9120 Compare October 3, 2022 04:50
@DilumAluthge DilumAluthge added the ci Continuous integration label Oct 3, 2022
@DilumAluthge DilumAluthge force-pushed the dpa/busybox-cache-julialang-org branch 2 times, most recently from 604ccb7 to ab0d96f Compare October 3, 2022 11:43
staticfloat
staticfloat requested changes Oct 3, 2022
View reviewed changes
@DilumAluthge DilumAluthge force-pushed the dpa/busybox-cache-julialang-org branch from ab0d96f to 8f51c0b Compare October 4, 2022 07:04
@DilumAluthge DilumAluthge merged commit 7f507e6 into master Oct 4, 2022
@DilumAluthge DilumAluthge deleted the dpa/busybox-cache-julialang-org branch October 4, 2022 14:49
@vtjnash
Copy link
Copy Markdown
Member

vtjnash commented Oct 4, 2022

Did this break CI? I am seeing:

Error During Test at C:\buildkite-agent\builds\win2k22-amdci6-5\julialang\julia-master\julia-2c969ea8c8\share\julia\test\testdefs.jl:21
  Got exception outside of a @test
  LoadError: UndefVarError: cache_output not defined
  Stacktrace:
    [1] _tryonce_download_from_cache(desired_url::String)
      @ Main.Test14Main_spawn C:\buildkite-agent\builds\win2k22-amdci6-5\julialang\julia-master\julia-2c969ea8c8\share\julia\test\spawn.jl:27
    [2] #1
      @ C:\buildkite-agent\builds\win2k22-amdci6-5\julialang\julia-master\julia-2c969ea8c8\share\julia\test\spawn.jl:42 [inlined]
    [3] (::Base.var"#90#92"{Base.var"#90#91#93"{Vector{Float64}, Nothing, Main.Test14Main_spawn.var"#1#2"{String}}})(; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Base .\error.jl:309
    [4] #90
      @ .\error.jl:291 [inlined]
    [5] download_from_cache(desired_url::String)
      @ Main.Test14Main_spawn C:\buildkite-agent\builds\win2k22-amdci6-5\julialang\julia-master\julia-2c969ea8c8\share\julia\test\spawn.jl:45

@staticfloat
Copy link
Copy Markdown
Member

staticfloat commented Oct 4, 2022

Whoops, expected failures strike again; we didn't notice the breakage because windows testers are already failing with the clipboard error. :/

DilumAluthge added a commit that referenced this pull request Oct 4, 2022
@DilumAluthge
Revert "If cache.julialang.org is down, try to download Busybox fro…
afa8885 
…m the upstream URL (#47015)"

This reverts commit 7f507e6.
staticfloat added a commit that referenced this pull request Oct 4, 2022
@staticfloat
Fix typo in test/spawn.jl
cc72ba7 
Fixes the error introduced by #47015 for windows tests.
@staticfloat staticfloat mentioned this pull request Oct 4, 2022
Merged
staticfloat added a commit that referenced this pull request Oct 4, 2022
@staticfloat
Fix typo in test/spawn.jl (#47049)
f056dbc 
Fixes the error introduced by #47015 for windows tests.
@IanButterworth IanButterworth mentioned this pull request Sep 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LilithHafner LilithHafner LilithHafner left review comments

@staticfloat staticfloat staticfloat approved these changes

Assignees

No one assigned

Labels

ci Continuous integration system:windows Affects only Windows test This change adds or pertains to unit tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@DilumAluthge @LilithHafner @vtjnash @staticfloat