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

Remove ru.yandex image & jdbc driver support #9221

Closed
wants to merge 1 commit into from

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 as a code owner 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