Skip to content

Pass null-terminated node ID for VM_RegisterClusterMessageReceiver and add test coverage#1708

Merged
zuiderkwast merged 12 commits intovalkey-io:unstablefrom
Nikhil-Manglore:module_callback_registration
Feb 20, 2025
Merged

Pass null-terminated node ID for VM_RegisterClusterMessageReceiver and add test coverage#1708
zuiderkwast merged 12 commits intovalkey-io:unstablefrom
Nikhil-Manglore:module_callback_registration

Conversation

@Nikhil-Manglore
Copy link
Member

@Nikhil-Manglore Nikhil-Manglore commented Feb 11, 2025

Closes #1656.

@codecov
Copy link

codecov bot commented Feb 11, 2025

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 71.14%. Comparing base (da3f1c6) to head (18117ba).
Report is 37 commits behind head on unstable.

Files with missing lines Patch % Lines
src/cluster_legacy.c 0.00% 3 Missing ⚠️
src/module.c 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1708      +/-   ##
============================================
+ Coverage     71.06%   71.14%   +0.08%     
============================================
  Files           121      123       +2     
  Lines         65254    65534     +280     
============================================
+ Hits          46371    46627     +256     
- Misses        18883    18907      +24     
Files with missing lines Coverage Δ
src/module.c 9.61% <0.00%> (+0.01%) ⬆️
src/cluster_legacy.c 85.87% <0.00%> (-0.39%) ⬇️

... and 32 files with indirect coverage changes

@Nikhil-Manglore
Copy link
Member Author

Looking into the CI Failure

Copy link
Contributor

@hpatro hpatro left a comment

Choose a reason for hiding this comment

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

Mostly LGTM.

@hpatro hpatro requested a review from zuiderkwast February 17, 2025 20:37
@Nikhil-Manglore Nikhil-Manglore changed the title Added test coverage for module callback registration for cluster message Null terminated the Node ID and added test coverage for module callback registration for cluster message Feb 19, 2025
@Nikhil-Manglore Nikhil-Manglore changed the title Null terminated the Node ID and added test coverage for module callback registration for cluster message Null-terminated Node ID and add test coverage for module cluster message callbacks Feb 19, 2025
Signed-off-by: Nikhil Manglore <[email protected]>
@hpatro hpatro changed the title Null-terminated Node ID and add test coverage for module cluster message callbacks Pass null-terminated node ID for VM_RegisterClusterMessageReceiver and add test coverage Feb 19, 2025
Signed-off-by: Nikhil Manglore <[email protected]>
Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Yeah, just some nits, then let's merge it.

@zuiderkwast zuiderkwast merged commit 206f7f6 into valkey-io:unstable Feb 20, 2025
51 checks passed
@Nikhil-Manglore Nikhil-Manglore deleted the module_callback_registration branch February 20, 2025 00:25
zuiderkwast pushed a commit that referenced this pull request Mar 18, 2025
…and add test coverage (#1708)

* Pass `sender_id` as `NULL` terminated string as part of
`ValkeyModuleClusterMessageReceiver` for ease of usage by the module(s).

* Implement test coverage described in #1656 and ensures that nodes in
the cluster module properly acknowledge a "DING" message by sending a
"DONG" response.

Closes #1656.

---------

Signed-off-by: Nikhil Manglore <[email protected]>
xbasel pushed a commit to xbasel/valkey that referenced this pull request Mar 27, 2025
…and add test coverage (valkey-io#1708)

* Pass `sender_id` as `NULL` terminated string as part of
`ValkeyModuleClusterMessageReceiver` for ease of usage by the module(s).

* Implement test coverage described in valkey-io#1656 and ensures that nodes in
the cluster module properly acknowledge a "DING" message by sending a
"DONG" response.

Closes valkey-io#1656.

---------

Signed-off-by: Nikhil Manglore <[email protected]>
xbasel pushed a commit to xbasel/valkey that referenced this pull request Mar 27, 2025
…and add test coverage (valkey-io#1708)

* Pass `sender_id` as `NULL` terminated string as part of
`ValkeyModuleClusterMessageReceiver` for ease of usage by the module(s).

* Implement test coverage described in valkey-io#1656 and ensures that nodes in
the cluster module properly acknowledge a "DING" message by sending a
"DONG" response.

Closes valkey-io#1656.

---------

Signed-off-by: Nikhil Manglore <[email protected]>
@zuiderkwast zuiderkwast moved this from To be backported to Unknown if backported in Valkey 8.1 Apr 7, 2025
murphyjacob4 pushed a commit to enjoy-binbin/valkey that referenced this pull request Apr 13, 2025
…and add test coverage (valkey-io#1708)

* Pass `sender_id` as `NULL` terminated string as part of
`ValkeyModuleClusterMessageReceiver` for ease of usage by the module(s).

* Implement test coverage described in valkey-io#1656 and ensures that nodes in
the cluster module properly acknowledge a "DING" message by sending a
"DONG" response.

Closes valkey-io#1656.

---------

Signed-off-by: Nikhil Manglore <[email protected]>
@zuiderkwast zuiderkwast moved this from Unknown if backported to Done in Valkey 8.1 Apr 17, 2025
@ranshid ranshid mentioned this pull request Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Add test coverage for module callback registration for cluster message via VM_RegisterClusterMessageReceiver

5 participants