Skip to content

ClickHouse uses new driver if it is available and version is compatible#6236

Merged
eddumelendez merged 3 commits intotestcontainers:mainfrom
trolley813:main
Feb 1, 2023
Merged

ClickHouse uses new driver if it is available and version is compatible#6236
eddumelendez merged 3 commits intotestcontainers:mainfrom
trolley813:main

Conversation

@trolley813
Copy link
Copy Markdown
Contributor

A replacement for #5666 (related to the now-closed #6232). Now, ClickHouseContainer picks up the new com.clickhouse.jdbc.ClickHouseDriver driver if it is available and the version tag is 20.7 or later (as supported by the driver), otherwise it falls back to the legacy ru.yandex.clickhouse.ClickHouseDriver driver.

@trolley813 trolley813 requested a review from a team November 24, 2022 17:11
}
}

private static boolean isNewDriverSupported(DockerImageName dockerImageName) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you may want to use ComparableVersion. See

new ComparableVersion(dockerImageName.getVersionPart()).isGreaterThanOrEqualTo("8.0.0");

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.

@eddumelendez Oh, that's great, thanks! Will fix it as soon as possible.

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.

Replaced with ComparableVersion.

@trolley813
Copy link
Copy Markdown
Contributor Author

Oh, I forgot to run spotless after making changes. (Actually, the only violation were unsorted imports.)
Now, it's probably ready.

@eddumelendez eddumelendez merged commit 80700c1 into testcontainers:main Feb 1, 2023
@eddumelendez
Copy link
Copy Markdown
Member

Thanks for your contribution @trolley813 ! This is now is main branch and it will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement]: Make ClickHouseContainer try to use new driver by default

2 participants