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

Update JDBC driver for MemSQL #7179

Merged
merged 1 commit into from
Mar 8, 2021
Merged

Update JDBC driver for MemSQL #7179

merged 1 commit into from
Mar 8, 2021

Conversation

findepi
Copy link
Member

@findepi findepi commented Mar 5, 2021

This fixes compatibility issue with recent MemSQL versions.

The driver previously used would fail to authenticate, with following
being printed in MemSQL logs

ERROR: ProcessHandshakeResponsePacket() failed. Sending back 1045: Access denied for user 'root'@'172.17.0.1' (using password: YES)

@cla-bot cla-bot bot added the cla-signed label Mar 5, 2021
@findepi

This comment has been minimized.

@findepi findepi force-pushed the findepi/memsql-up branch from f28a4aa to 4d6f1bf Compare March 5, 2021 16:55
This fixes compatibility issue with recent MemSQL versions.

The driver previously used would fail to authenticate, with following
being printed in MemSQL logs

    ERROR: ProcessHandshakeResponsePacket() failed. Sending back 1045: Access denied for user 'root'@'172.17.0.1' (using password: YES)
@findepi findepi force-pushed the findepi/memsql-up branch from 4d6f1bf to 58a4b71 Compare March 6, 2021 20:26
@findepi
Copy link
Member Author

findepi commented Mar 6, 2021

CI #6723

@findepi findepi requested review from losipiuk and sopel39 March 6, 2021 21:55
@@ -31,6 +31,7 @@
private static final String MEM_SQL_LICENSE = requireNonNull(System.getProperty("memsql.license"), "memsql.license is not set");

public static final String DEFAULT_TAG = "memsql/cluster-in-a-box:centos-7.1.13-11ddea2a3a-3.0.0-1.9.0";
public static final String LATEST_TESTED_TAG = "memsql/cluster-in-a-box:centos-7.3.4-d596a2867a-3.2.4-1.10.1";
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to add a test which checks if latest did not get stale?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is current pattern and see #5804 for more info.

We don't automatically support newer MemSQL version when one is released, so a test verifying our Latest is truly latest could start failing any time, and it could be hard to make it fixed, if there were any breaking changes.

We should read "Latest" as "Latest Tested" where "Tested" is omitted for brevity.

@findepi findepi merged commit 2c6a5f9 into master Mar 8, 2021
@findepi findepi deleted the findepi/memsql-up branch March 8, 2021 09:50
@findepi findepi added this to the 354 milestone Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants