added client change server event #182
Conversation
|
interesting PR, let's chat this week. Also please remove .DS_store file. |
valkeymodule-rs-macros/Cargo.lock
Outdated
There was a problem hiding this comment.
@sachinvmurthy - please remove the .lock file in the macros but add back the one in the root
examples/server_events.rs
Outdated
| @@ -30,6 +32,21 @@ fn cron_event_handler(_ctx: &Context, _hz: u64) { | |||
| NUM_CRONS.fetch_add(1, Ordering::SeqCst); | |||
| } | |||
|
|
|||
| #[client_changed_event_handler] | |||
| fn client_changed_event_handler(ctx: &Context, client_event: ClientChangeSubevent){ | |||
There was a problem hiding this comment.
Thanks for adding this to the example module. Could you add a test case for this as well?
There was a problem hiding this comment.
thanks :D! will add the test case.
| @@ -30,6 +32,21 @@ fn cron_event_handler(_ctx: &Context, _hz: u64) { | |||
| NUM_CRONS.fetch_add(1, Ordering::SeqCst); | |||
| } | |||
|
|
|||
| #[client_changed_event_handler] | |||
| fn client_changed_event_handler(ctx: &Context, client_event: ClientChangeSubevent){ | |||
| if let ClientChangeSubevent::Connected = client_event { | |||
There was a problem hiding this comment.
We can also check and handle the disconnects in this example - by decrementing the counter for example. We can then validate disconnection handling as well from the integration test
|
@sachinvmurthy Looks like you forgot to signoff on the commit. You can follow the instructions here: https://github.com/valkey-io/valkeymodule-rs/pull/182/checks?check_run_id=38259568280 Also, Could we please add some details to the PR description when you get time? |
Signed-off-by: sacvenka <[email protected]>
sachinvmurthy
force-pushed
the
client_change_event
branch
from
7c07102 to
e576c19
Compare
Signed-off-by: sacvenka <[email protected]>
Signed-off-by: sacvenka <[email protected]>
Signed-off-by: sacvenka <[email protected]>
Signed-off-by: sacvenka <[email protected]>
sachinvmurthy
force-pushed
the
client_change_event
branch
from
e576c19 to
c286c7c
Compare
| @@ -44,6 +44,12 @@ impl Context { | |||
| ValkeyString::from_redis_module_string(self.ctx, client_username) | |||
| } | |||
|
|
|||
| pub fn get_client_username_by_id(&self, client_id: u64) -> ValkeyString { | |||
There was a problem hiding this comment.
@dmitrypol added this as an additional functionality to get username just by id.
| let res: i64 = redis::cmd("num_connects").query(&mut con)?; | ||
|
|
||
| assert_eq!(res, 0); | ||
|
|
There was a problem hiding this comment.
test for num_disconnects?
| ) { | ||
| let client_change_sub_event = if subevent == raw::REDISMODULE_SUBEVENT_CLIENT_CHANGE_CONNECTED { | ||
| ClientChangeSubevent::Connected | ||
| } else { |
There was a problem hiding this comment.
should we use REDISMODULE_SUBEVENT_CLIENT_CHANGE_DISCONNECTED as well?
This PR enhances the server_events module by adding support for handling client change events.
Specifically, it introduces the
ClientChangeSubeventenum to represent client connection and disconnection events, adds a new event handler functionclient_change_event_callbackto process these events, and updates theregister_server_eventsfunction to include the registration of client change events. This allows the module to track and respond to client connection state changes, improving observability and control over client interactions.