Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,32 @@ public String selectSparkVersion(SparkVersionSelector selector) throws IllegalAr
if (!selector.latest) {
throw new IllegalArgumentException("spark versions query returned multiple results");
}
versions.sort((v1, v2) -> SemVer.parse(v2).compareTo(SemVer.parse(v1)));
versions.sort(ClustersExt::compareSparkVersionsDescending);
}
return versions.get(0);
}

/**
* Compares two Spark runtime keys so that the latest version sorts first. Mirrors the
* databricks-sdk-go behavior (golang.org/x/mod/semver.Compare), where keys that are not valid
* SemVer (for example "v18.x-scala2.13") are treated as lowest priority instead of throwing. This
* ensures a single unparseable runtime key returned by the API cannot break version selection.
*/
private static int compareSparkVersionsDescending(String v1, String v2) {
SemVer s1 = SemVer.parseOrNull(v1);
SemVer s2 = SemVer.parseOrNull(v2);
if (s1 == null && s2 == null) {
return 0;
}
if (s1 == null) {
return 1; // v1 is unparseable: sort it after v2
}
if (s2 == null) {
return -1; // v2 is unparseable: keep v1 before v2
}
return s2.compareTo(s1); // descending order: latest first
}

public String selectNodeType(NodeTypeSelector selector) {
// Logic ported from
// https://github.com/databricks/databricks-sdk-go/blob/main/service/clusters/node_type.go
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ public static SemVer parse(String v) {
m.group("build"));
}

/**
* Parses the given version string, returning {@code null} instead of throwing when it is not a
* valid SemVer. Useful when sorting collections that may contain non-SemVer values (for example
* Spark runtime keys such as "v18.x-scala2.13").
*/
public static SemVer parseOrNull(String v) {
try {
return parse(v);
} catch (IllegalArgumentException e) {
return null;
}
}

@Override
public int compareTo(SemVer other) {
if (other == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,34 @@ void sparkVersionWithSparkVersionParameter() {
clustersExt.selectSparkVersion(new SparkVersionSelector().withSparkVersion("3.4.1"));
assertEquals("13.3.x-scala2.12", sparkVersion);
}

private GetSparkVersionsResponse testGetSparkVersionsWithUnparseableKey() {
Collection<SparkVersion> versions = new ArrayList<>();
versions.add(
new SparkVersion()
.setName("14.3 LTS (includes Apache Spark 3.5.0, Scala 2.12)")
.setKey("14.3.x-scala2.12"));
versions.add(
new SparkVersion()
.setName("15.4 LTS (includes Apache Spark 3.5.0, Scala 2.12)")
.setKey("15.4.x-scala2.12"));
// Non-SemVer runtime key returned by the API. Sorting this with SemVer.parse() previously threw
// "Not a valid SemVer: v18.x-scala2.13" and broke selection for every caller.
versions.add(
new SparkVersion()
.setName("18.x (includes Apache Spark 4.0.0, Scala 2.13)")
.setKey("v18.x-scala2.13"));
return new GetSparkVersionsResponse().setVersions(versions);
}

@Test
void selectLatestSparkVersionIgnoresUnparseableKey() {
ClustersExt clustersExt = new ClustersExt(clustersMock);
Mockito.doReturn(testGetSparkVersionsWithUnparseableKey()).when(clustersMock).sparkVersions();

// Must not throw on the non-SemVer key, and must return the latest parseable runtime - matching
// databricks-sdk-go, which ranks unparseable keys lowest rather than failing.
String sparkVersion = clustersExt.selectSparkVersion(new SparkVersionSelector().withLatest());
assertEquals("15.4.x-scala2.12", sparkVersion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,21 @@ void parseTest() {
int compareResult = parsedSemVer.compareTo(expectedSemVer);
assertEquals(0, compareResult);
}

@Test
void parseOrNullReturnsNullForInvalid() {
// Spark runtime key shape that crashed selectSparkVersion(): only two version segments and a
// leading "v", which is not a valid SemVer.
assertNull(SemVer.parseOrNull("v18.x-scala2.13"));
assertNull(SemVer.parseOrNull("not-a-version"));
assertNull(SemVer.parseOrNull(null));
assertNull(SemVer.parseOrNull(""));
}

@Test
void parseOrNullParsesValid() {
SemVer parsed = SemVer.parseOrNull("v1.2.3-alpha+build-20230510");
assertNotNull(parsed);
assertEquals(0, parsed.compareTo(new SemVer(1, 2, 3, "alpha", "build-20230510")));
}
}
Loading