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.

Sign up for GitHub

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

Jump to bottom

Add CLIENT ID and CLIENT GETNAME commands. #98

Merged
merged 6 commits into from
Feb 21, 2024
Merged

Add CLIENT ID and CLIENT GETNAME commands. #98

merged 6 commits into from
Feb 21, 2024

Conversation

Yury-Fridlyand
Copy link

@Yury-Fridlyand Yury-Fridlyand commented Feb 15, 2024 •
edited
Loading

acarbonetto
acarbonetto reviewed Feb 20, 2024
View reviewed changes
java/client/src/main/java/glide/api/commands/ConnectionManagementClusterCommands.java Outdated Show resolved Hide resolved
java/client/src/main/java/glide/api/RedisClusterClient.java Outdated
@@ -118,4 +122,40 @@ public CompletableFuture<ClusterValue<String>> info(
return commandManager.submitNewCommand(
Info, options.toArgs(), route, response -> ClusterValue.of(handleObjectResponse(response)));
}

/** {@inheritDoc} The command will be routed a random node. */
Copy link

Choose a reason for hiding this comment

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

Do we want to follow this throughout - whenever we have a command with an extra comment when used in clustermode?

Problem I have is it breaks convention and we now have documentation in the implementation files.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in cf519a0

java/client/src/main/java/glide/api/RedisClusterClient.java Outdated
if (route.isSingleNodeRoute()) {
return ClusterValue.ofSingleValue(handleObjectOrNullResponse(response));
}
if (response.hasConstantResponse()) {
Copy link

Choose a reason for hiding this comment

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

We should have a test for this in java/client/src/test/java/glide/api/RedisClusterClientTest.java

Copy link
Author

Choose a reason for hiding this comment

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

Tested in custom_command_returns_single_value_on_constant_response (a part of another PR), already merged on upstream

java/integTest/src/test/java/glide/cluster/CommandTests.java Outdated
"Modules",
"Errorstats",
"Cluster",
"Keyspace");
Copy link

Choose a reason for hiding this comment

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

Did this change...? I don't understand why you have so many spotless differences in this file.

Copy link

Choose a reason for hiding this comment

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

Oh... this is the file that failed spotless before. This will get fixed by rebase, right?

Copy link
Author

Choose a reason for hiding this comment

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

Rebased and cleaned

@Yury-Fridlyand Yury-Fridlyand force-pushed the java/dev_yuryf_client_id_getname branch from e220cc5 to c31877e Compare February 20, 2024 22:08
@Yury-Fridlyand
javajavadocdoc
817edf6 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
Add interface inheritance to inherit & extend javadocs.
cf519a0 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
Minor fixes.
4f3f614 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
javajavadocdoc
88649d9 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
tense
7d6c220 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand Yury-Fridlyand merged commit 68ba1a0 into java/integ_yuryf_client_id_getname Feb 21, 2024
10 checks passed
@Yury-Fridlyand Yury-Fridlyand deleted the java/dev_yuryf_client_id_getname branch February 21, 2024 20:19
SanHalacogluImproving pushed a commit that referenced this pull request Feb 27, 2024
@Yury-Fridlyand @SanHalacogluImproving
Add CLIENT ID and CLIENT GETNAME commands. (#98)
50ffec9 
* Add `CLIENT ID` and `CLIENT GETNAME` commands.

Signed-off-by: Yury-Fridlyand <[email protected]>
SanHalacogluImproving pushed a commit that referenced this pull request Mar 5, 2024
@Yury-Fridlyand @SanHalacogluImproving
Add CLIENT ID and CLIENT GETNAME commands. (#98)
93e452c 
* Add `CLIENT ID` and `CLIENT GETNAME` commands.

Signed-off-by: Yury-Fridlyand <[email protected]>
SanHalacogluImproving added a commit that referenced this pull request Mar 5, 2024
@Yury-Fridlyand @acarbonetto @SanHalacogluImproving
Add CLIENT ID and CLIENT GETNAME commands. (valkey-io#1014)
0ee5ab0 
* Add `CLIENT ID` and `CLIENT GETNAME` commands. (#98)

Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]>
Co-authored-by: SanHalacogluImproving <[email protected]>
cyip10 pushed a commit that referenced this pull request Jun 24, 2024
@Yury-Fridlyand @acarbonetto
@SanHalacogluImproving @cyip10
Add CLIENT ID and CLIENT GETNAME commands. (valkey-io#1014)
003e8ec 
* Add `CLIENT ID` and `CLIENT GETNAME` commands. (#98)

Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]>
Co-authored-by: SanHalacogluImproving <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acarbonetto acarbonetto acarbonetto approved these changes

@aaron-congo aaron-congo Awaiting requested review from aaron-congo

@jonathanl-bq jonathanl-bq Awaiting requested review from jonathanl-bq

@Elen-Ghulam Elen-Ghulam Awaiting requested review from Elen-Ghulam

@SanHalacogluImproving SanHalacogluImproving Awaiting requested review from SanHalacogluImproving

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Yury-Fridlyand @acarbonetto