[internal review] Go: implement standalone custom command #152
Conversation
aaron-congo
changed the title
aaron-congo
changed the title
go/api/commands.go
Outdated
| // | ||
| // For example, to return a list of all pub/sub clients: | ||
| // | ||
| // client.CustomCommand([]string{"CLIENT", "LIST","TYPE", "PUBSUB"}) |
There was a problem hiding this comment.
can we add a response to this example? what do we do with the returned payload/error?
There was a problem hiding this comment.
The other clients use this same example and similarly do not do anything with the response. Let me know if you want me to add one here anyways though. What should I do with the response? Just print it like this? Or should I show that it needs to be casted from interface{} to string if used for anything other than Printf (which accepts interface{} parameters)
result, err := client.CustomCommand([]string{"CLIENT", "LIST","TYPE", "PUBSUB"})
if err != nil {
log.Fatal("error executing command: ", err)
}
fmt.Printf("result: %s", result)
| Close() | ||
| } | ||
|
|
||
| type payload struct { |
There was a problem hiding this comment.
- capitalize
- which payload? response? connection? command? consider renaming
- add a doc
| // TODO: call lib.rs function to free response | ||
| response := C.GoString(cResponse) | ||
| resultChannel := *(*chan payload)(channelPtr) | ||
| resultChannel <- payload{value: response, error: nil} |
| resultChannel := make(chan payload) | ||
| resultChannelPtr := uintptr(unsafe.Pointer(&resultChannel)) | ||
|
|
||
| C.command(client.coreClient, C.uintptr_t(resultChannelPtr), requestType, C.uintptr_t(len(args)), &cArgs[0]) |
There was a problem hiding this comment.
You changed callback singature to take channel pointer as void *. Should be here second argument be void * too?
| @@ -20,3 +22,18 @@ func NewRedisClient(config *RedisClientConfiguration) (*RedisClient, error) { | |||
|
|
|||
| return &RedisClient{client}, nil | |||
| } | |||
|
|
|||
| // CustomCommand executes a single command, specified by args, without checking inputs. Every part of the command, including | |||
There was a problem hiding this comment.
Consider creating a list of interfaces as we did in java client.
Is it possible in go to split doc and implementation into different files?
| // | ||
| // client.CustomCommand([]string{"CLIENT", "LIST","TYPE", "PUBSUB"}) | ||
| func (client *RedisClient) CustomCommand(args []string) (interface{}, error) { | ||
| return client.executeCommand(C.CustomCommand, args) |
There was a problem hiding this comment.
add non-null and non-empty check for args
| * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 | ||
| */ | ||
| #![deny(unsafe_op_in_unsafe_fn)] | ||
| // TODO: uncomment |
| ) -> Result<Vec<String>, Utf8Error> { | ||
| from_raw_parts(data, len) | ||
| .iter() | ||
| .map(|arg| CStr::from_ptr(*arg).to_str().map(ToString::to_string)) |
| use std::slice::from_raw_parts; | ||
| use std::str::Utf8Error; | ||
|
|
||
| pub unsafe fn convert_double_pointer_to_vec( |
There was a problem hiding this comment.
add docstring with # Safety section or remove unsafe
Also, should not be pub I think
| cmd | ||
| } | ||
|
|
||
| use std::slice::from_raw_parts; |
| } | ||
|
|
||
| #[no_mangle] | ||
| pub unsafe extern "C" fn command( |
There was a problem hiding this comment.
add docstring with # Safety section
|
Closing as this is superseded by another PR upstream (valkey-io#1937) |
Note: