Skip to content

Single build path [MOD-7480]#4935

Merged
alonre24 merged 66 commits intomasterfrom
single_build_path
Sep 16, 2024
Merged

Single build path [MOD-7480]#4935
alonre24 merged 66 commits intomasterfrom
single_build_path

Conversation

@alonre24
Copy link
Collaborator

@alonre24 alonre24 commented Aug 18, 2024

Describe the changes in the pull request

Complete the unified build for the module with the coordinator's component which is now an integral part of the module. This includes:

  • Removing the option to build the module without a coordinator (COORD=0), and removing the RS_COORDINTOR build flag (which is now always true).
  • As for the LITE flavor - it is still possible to build an artifact under the name searchlight for backward compatibility, but the behavior will be the same as the non-light module (that is, the coordinator will take place and start lazily in a clustered environment).
  • Move coord/module.c file into the module's module.c (module's entry point where RedisModule_OnLoad function should be) and resolve dependencies (link the entire module against hiredis/sds.h rather than rmutil/sds.h, and let the CPP tests use it but wrapping it with extern C).
  • Let the cmakelists.txt file in the project root dir be the single entry point from which the modules (and unit test) artifacts are built (having the cmakelists.txt under coord a subdirectory only, not an entry point as it was).
  • Move coordinator unit tests into the ctests and cpptests dirs and remove test files that are outdated and look more like POCs than tests
  • Move ramp files into pack dir and rename them according to their purpose (ce, lite, enterprise)
  • Remove redundant RSBuildType enum and build-info dir, and use IsEnterprise() call in runtime to determine the cluster build type instead.
  • Bump libuv version to the latest - 1.48.0 (this also removes warnings in the code build...)

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

@alonre24 alonre24 requested a review from DvirDukhan August 19, 2024 13:26
@alonre24 alonre24 marked this pull request as ready for review August 19, 2024 13:36
@codecov
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

Attention: Patch coverage is 90.33898% with 114 lines in your changes missing coverage. Please review.

Project coverage is 86.30%. Comparing base (4a67a87) to head (0d88e88).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/module.c 90.33% 114 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4935      +/-   ##
==========================================
+ Coverage   86.18%   86.30%   +0.12%     
==========================================
  Files         193      190       -3     
  Lines       34704    34593     -111     
==========================================
- Hits        29908    29856      -52     
+ Misses       4796     4737      -59     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alonre24 alonre24 changed the title Single build path Single build path [MOD-7480] Aug 20, 2024
Copy link

@DvirDukhan DvirDukhan left a comment

Choose a reason for hiding this comment

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

Nice!
More questions than comments

@alonre24 alonre24 requested a review from GuyAv46 August 28, 2024 12:19
Copy link
Collaborator

@GuyAv46 GuyAv46 left a comment

Choose a reason for hiding this comment

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

🧼 🧽

kei-nan and others added 4 commits August 28, 2024 15:15
* * initial commit

* * Use RediSearch branch mirroring fork

* * Code Review - Round #1
…4972)

change the number of maxPending requests when changing the number of connections
@alonre24 alonre24 requested review from GuyAv46 and removed request for GuyAv46 September 15, 2024 09:21
@alonre24 alonre24 requested a review from GuyAv46 September 15, 2024 14:33
@alonre24 alonre24 enabled auto-merge September 15, 2024 15:31
GuyAv46
GuyAv46 previously approved these changes Sep 15, 2024
@alonre24 alonre24 added this pull request to the merge queue Sep 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 15, 2024
@alonre24 alonre24 added this pull request to the merge queue Sep 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Sep 15, 2024
@alonre24 alonre24 added this pull request to the merge queue Sep 16, 2024
Merged via the queue into master with commit ba07994 Sep 16, 2024
@alonre24 alonre24 deleted the single_build_path branch September 16, 2024 08:12
@github-actions
Copy link

Backport failed for 8.0, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 8.0
git worktree add -d .worktree/backport-4935-to-8.0 origin/8.0
cd .worktree/backport-4935-to-8.0
git switch --create backport-4935-to-8.0
git cherry-pick -x ba07994f5e5a991f93e27df556e30e9a2de40b95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants