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

Fetch server version using info command #2258

Merged

Conversation

prateek-kumar-improving
Copy link
Collaborator

Get server version using info command in TestConfiguration.java

Signed-off-by: Prateek Kumar <[email protected]>
Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

You probably need to run linter before commit

String[] serverSectionArray = infoResponse.split("\n");

for (int i = 0; i < serverSectionArray.length; i++) {
if (serverSectionArray[i].contains("redis_version")) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check for valkey version first

127.0.0.1:7000> info server
# Server
redis_version:7.2.4
server_name:valkey
valkey_version:7.9.240

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

}

} catch (Exception e) {
throw new RuntimeException(e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a message, e.g. "failed to get server version"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.


static {
try {
standaloneClient = GlideClient.createClient(commonClientConfig().build()).get();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
standaloneClient = GlideClient.createClient(commonClientConfig().build()).get();
var standaloneClient = GlideClient.createClient(commonClientConfig().build()).get();

No need static lifetime for the client

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

private static final GlideClient standaloneClient;
public static final Semver SERVER_VERSION;

static {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a function to TestUtilities which runs info and parses the response. The function has one argument - client.
Then static block will call that function. Add @SneakyThrows to the function - we don't care about exceptions.

This could be used in some specific tests where we could have servers of different versions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.
Moved function to TestUtilities.

@prateek-kumar-improving
Copy link
Collaborator Author

prateek-kumar-improving commented Sep 10, 2024

You probably need to run linter before commit

Updated. Used these 2 commands for linter check
./gradlew :spotlessCheck
./gradlew :spotlessApply

Copy link
Collaborator

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

Please clean up gradle script now

@prateek-kumar-improving
Copy link
Collaborator Author

Please clean up gradle script now

Creating a separate PR for this.

@GumpacG GumpacG merged commit b081eec into valkey-io:main Sep 10, 2024
13 checks passed
@GumpacG GumpacG deleted the java-get-server-version-using-info-command branch September 10, 2024 23:55
janhavigupta007 pushed a commit to janhavigupta007/glide-for-redis that referenced this pull request Sep 11, 2024
* Fetch server version using info command

Signed-off-by: Prateek Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants