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

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

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Oct 3, 2022

  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
@DilumAluthge DilumAluthge force-pushed the dpa/busybox-cache-julialang-org branch from 6552cd2 to 6781838 Compare October 3, 2022 01:56
@DilumAluthge
Copy link
Member Author

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

Copy link
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
Member Author

DilumAluthge commented Oct 3, 2022

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
Member

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

@DilumAluthge
Copy link
Member Author

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
test/spawn.jl Outdated Show resolved Hide resolved
@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
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
Member

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
staticfloat added a commit that referenced this pull request Oct 4, 2022
Fixes the error introduced by #47015 for windows tests.
staticfloat added a commit that referenced this pull request Oct 4, 2022
Fixes the error introduced by #47015 for windows tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration system:windows Affects only Windows test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants