Automatically determine appropriate URL based off platform in VersionDownloadSource - #96
Merged
Merged
Conversation
bodowd
marked this pull request as draft
August 5, 2026 11:56
bodowd
marked this pull request as ready for review
August 5, 2026 12:05
bodowd
force-pushed
the
automatically-determine-url-for-sys-arch
branch
from
August 5, 2026 12:13
9b4feba to
8f2b025
Compare
BaurzhanSakhariev
approved these changes
Aug 5, 2026
bodowd
commented
Aug 5, 2026
| @Override | ||
| public URL downloadUrl() throws MalformedURLException { | ||
| return new URL(String.format(Locale.ENGLISH, VERSION_DOWNLOAD_URL, version)); | ||
| return buildDownloadUrl(this.version, platform( |
Contributor
Author
There was a problem hiding this comment.
This bit of indirection made it easier to write unit tests on buildDownloadUrl and platform
bodowd
commented
Aug 5, 2026
| System.getProperty("os.arch"))); | ||
| } | ||
|
|
||
| static URL buildDownloadUrl(String version, String platform) throws MalformedURLException { |
Contributor
Author
There was a problem hiding this comment.
This and platform could potentially be in Utils and re-used for other tests like ShutdownTest. What do you think?
There was a problem hiding this comment.
Could be done later when/if we adjust ShutdownTest to be able to run locally from aarch64_mac
…DownloadSource Currently, downloading the cratedb releases assumes the following URL "https://cdn.crate.io/downloads/releases/crate-%s.tar.gz" However, this will not work if the user is running on aarch64_mac, and potentially other platforms. We now check the architecture of the system running the code and then determine the appropriate URL for downloading the desired cratedb release.
bodowd
force-pushed
the
automatically-determine-url-for-sys-arch
branch
from
August 5, 2026 12:30
8f2b025 to
467b8de
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the changes / Why this is an improvement
Related to this issue: crate/jmx_exporter#102
Related to PR: crate/jmx_exporter#104 so that jmx_exporter can simply pass a desired CrateDB version for download to crate-java-testing. This PR makes it easier to test different releases on jmx_exporter from different systems, not only Linux.
Currently,
CrateTestCluster.fromVersionwill download fromhttps://cdn.crate.io/downloads/releases/crate-%s.tar.gzHowever, if the client is running on aarch64_mac, for example, the releases at this URL are not suitable for the client's system. There are releases for specific architectures that should be used instead. For example: https://cdn2.crate.io/downloads/releases/nightly/aarch64_mac/
Currently, the client must work around this by browsing https://cdn2.crate.io/downloads/releases/ to find the url to the release they want and then call
CrateTestCluster.fromURL.This PR allows the client to simply pass the desired version they want to download and it abstracts away figuring out the appropriate CrateDB release to download.
Note
This PR does not yet address making tests run locally from aarch64_mac. Further changes would be necessary for that, for example here:
crate-java-testing/src/test/java/io/crate/testing/ShutdownTest.java
Lines 9 to 20 in 67b63a4
This still has a hardcoded URL. The workaround in
DEVELOP.rstis still necessary if developers want to run local tests on macosChecklist