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

Go: Implement ClientId #3077

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Go: Implement ClientId #3077

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e4e90a1
Implement CLientID
EdricCua Feb 5, 2025
f1a2de4
Implement CLientID
EdricCua Feb 5, 2025
b29dff9
Updates from main
EdricCua Feb 5, 2025
4b55072
Fix review comment
EdricCua Feb 5, 2025
dfa52dd
Fix review comment
EdricCua Feb 5, 2025
60bf8f1
Fix review comment
EdricCua Feb 5, 2025
92d55ad
Merge branch 'main' into Go-Implement-ClientId
Yury-Fridlyand Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go/api/connection_management_cluster_commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ type ConnectionManagementClusterCommands interface {
PingWithOptions(pingOptions options.ClusterPingOptions) (string, error)

EchoWithOptions(echoOptions options.ClusterEchoOptions) (ClusterValue[string], error)

ClientIdWithOptions(routeOptions options.RouteOption) (ClusterValue[int64], error)
}
44 changes: 44 additions & 0 deletions go/api/glide_cluster_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,3 +366,47 @@ func (client *GlideClusterClient) EchoWithOptions(echoOptions options.ClusterEch
}
return createClusterSingleValue[string](data), nil
}

// Gets the current connection id.
// The command will be routed a random node, unless `Route` in `routeOptions` is provided.
//
// Parameters:
//
// message - The provided message.
EdricCua marked this conversation as resolved.
Show resolved Hide resolved
//
// Return value:
//
// The id of the client.
//
// Example:
//
// route := config.Route(config.RandomRoute)
// opts = options.RouteOption{Route: route}
// response, err = client.ClientIdWithOptions(opts)
// if err != nil {
// // handle error
// }
// for node, data := range response {
// fmt.Printf("%s node returned %s\n", node, data)
// }
//
// [valkey.io]: https://valkey.io/commands/client-id/
func (client *GlideClusterClient) ClientIdWithOptions(opts options.RouteOption) (ClusterValue[int64], error) {
response, err := client.executeCommandWithRoute(C.ClientId, []string{}, opts.Route)
if err != nil {
return createEmptyClusterValue[int64](), err
}
if opts.Route != nil &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can remove the nil check here as the isMultiNode function has default implementation in request_routing_config.go :-

type Route interface {
	ToRoutesProtobuf() (*protobuf.Routes, error)
	IsMultiNode() bool
}

type notMultiNode struct{}

func (*notMultiNode) IsMultiNode() bool { return false }

Can you check once this ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can a user call ClientIdWithOptions(nil)? If yes, we can remove this nil check. If no, we need to adjust API of this command or RouteOption to make route parameter optional.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @Yury-Fridlyand and @omangesg, I have to do opts.Route != nil to avoid error when doing the IsMultiNode() check.

func (route SimpleNodeRoute) IsMultiNode() bool {
return route != RandomRoute
}

(opts.Route).IsMultiNode() {
data, err := handleStringIntMapResponse(response)
if err != nil {
return createEmptyClusterValue[int64](), err
}
return createClusterMultiValue[int64](data), nil
}
data, err := handleIntResponse(response)
if err != nil {
return createEmptyClusterValue[int64](), err
}
return createClusterSingleValue[int64](data), nil
}
27 changes: 27 additions & 0 deletions go/api/response_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,3 +1160,30 @@ func handleTimeClusterResponse(response *C.struct_CommandResponse) (ClusterValue
}
return createClusterSingleValue(data), nil
}

func handleStringIntMapResponse(response *C.struct_CommandResponse) (map[string]int64, error) {
defer C.free_command_response(response)

typeErr := checkResponseType(response, C.Map, false)
if typeErr != nil {
return nil, typeErr
}

data, err := parseMap(response)
if err != nil {
return nil, err
}
aMap := data.(map[string]interface{})

converted, err := mapConverter[int64]{
nil, false,
}.convert(aMap)
if err != nil {
return nil, err
}
result, ok := converted.(map[string]int64)
if !ok {
return nil, &errors.RequestError{Msg: fmt.Sprintf("unexpected type of map: %T", converted)}
}
return result, nil
}
32 changes: 32 additions & 0 deletions go/integTest/cluster_commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package integTest

import (
"fmt"
"strings"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -298,3 +299,34 @@ func (suite *GlideTestSuite) TestEchoCluster() {
assert.Contains(t, strings.ToLower(messages), strings.ToLower("hello"))
}
}

func (suite *GlideTestSuite) TestClientIdCluster() {
client := suite.defaultClusterClient()
t := suite.T()

// echo with option or with multiple options without route
opts := options.RouteOption{Route: nil}
response, err := client.ClientIdWithOptions(opts)
fmt.Println("response: ", response)
fmt.Println("err: ", err)
assert.NoError(t, err)
assert.True(t, response.IsSingleValue())

// same sections with random route
route := config.Route(config.RandomRoute)
opts = options.RouteOption{Route: route}
response, err = client.ClientIdWithOptions(opts)
fmt.Println("response: ", response)
fmt.Println("err: ", err)
EdricCua marked this conversation as resolved.
Show resolved Hide resolved
assert.NoError(t, err)
assert.True(t, response.IsSingleValue())

// default sections, multi node route
route = config.Route(config.AllPrimaries)
opts = options.RouteOption{Route: route}
response, err = client.ClientIdWithOptions(opts)
fmt.Println("response: ", response)
fmt.Println("err: ", err)
assert.NoError(t, err)
assert.True(t, response.IsMultiValue())
}
Loading