Skip to content

Comments

Remove ru.yandex image & jdbc driver support#9221

Closed
mzitnik wants to merge 1 commit intotestcontainers:mainfrom
mzitnik:update-jdbc-version
Closed

Remove ru.yandex image & jdbc driver support#9221
mzitnik wants to merge 1 commit intotestcontainers:mainfrom
mzitnik:update-jdbc-version

Conversation

@mzitnik
Copy link

@mzitnik mzitnik commented Sep 12, 2024

We are updating the ClickHouse module with a new ClickHouse image and removing the ru.yandex image and old JDBC driver.

@mzitnik mzitnik requested a review from a team September 12, 2024 12:13
@kiview
Copy link
Member

kiview commented Sep 17, 2024

Why do we need to make changes to the deprecated class org.testcontainers.containers.ClickHouseContainer?
Since we already have org.testcontainers.clickhouse.ClickHouseContainer, whch only supports the new image, wouldn't the next logical step to instead remove the deprecated class org.testcontainers.containers.ClickHouseContainer?

And I guess we can keep the change for the JDBC driver either way.

@mzitnik
Copy link
Author

mzitnik commented Sep 17, 2024

Since I do not know who is using that did not want to break the code
Just making sure we announced that the deprecated class would be removed in the next release?

@eddumelendez
Copy link
Member

org.testcontainers.containers.ClickHouseContainer is deprecated and the changes submitted are breaking changes for those still using it. If you are looking for to use only clickhouse/clickhouse-server image then move to org.testcontainers.clickhouse.ClickHouseContainer. We have an open PR #8738 that moves the ClickHouseProvider to use clickhouse/clickhouse-server instead of the previous one but it has not been merged because the existing implementation will break the behavior for existing users.

I'd suggest to discuss the ideas and look into existing issues and open PRs before submitting a PR. We know you have to invest time in order to make those changes.

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.

3 participants