Extract the scripting engine code from the functions unit #1312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rjd15372
force-pushed
the
refactor-engine-1261
branch
from
40bcded to
e8db485
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #1312 +/- ##
============================================
+ Coverage 70.83% 70.91% +0.07%
============================================
Files 120 121 +1
Lines 64911 65079 +168
============================================
+ Hits 45982 46152 +170
+ Misses 18929 18927 -2
|
|
@rjd15372 You could point the merge branch as your initial branch. It will show only the diff then. |
rjd15372
force-pushed
the
refactor-engine-1261
branch
5 times, most recently
from
990447a to
54eb99f
Compare
rjd15372
force-pushed
the
refactor-engine-1261
branch
from
54eb99f to
1ae23e8
Compare
rjd15372
force-pushed
the
refactor-engine-1261
branch
2 times, most recently
from
d095057 to
375d681
Compare
This commit creates a new unit for the scripting engine code by extracting the existing code from the functions unit. We're doing this refactor to prepare the code for runnning the `EVAL` command using different scripting engines. Signed-off-by: Ricardo Dias <[email protected]>
rjd15372
force-pushed
the
refactor-engine-1261
branch
from
375d681 to
125394c
Compare
…eate library callback Instead of using C strings to return the errors that may be happen during the code parsing of a scripting engine, use `robj` strings. This commit also removes the `valkey_asprintf` function, since it's not used in any place anymore. Signed-off-by: Ricardo Dias <[email protected]>
|
@zuiderkwast @PingXie this refactor is ready for review. |
PingXie
reviewed
Dec 24, 2024
There was a problem hiding this comment.
I took a quick look. I'd suggest renaming "engine" to "scripting engine" globally first. We have been using "engine", "core engine", and "Valkey server" interchangeably in our daily conversations, documentation, and the code. We should qualify this new "engine" as "scripting engine" to avoid confusion.
Sounds good 👍 |
Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Ricardo Dias <[email protected]>
zuiderkwast
reviewed
Jan 2, 2025
* renames the prefix of scripting engine functions from `engine` to `scriptingEngine` * fixes scripting_engine header file top macro name * reorders source file list in cmake Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Ricardo Dias <[email protected]>
* Changed return type of `engineIterCallback` to `void` * removed `exit(1)` call after `serverPanic` Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Ricardo Dias <[email protected]>
zuiderkwast
reviewed
Jan 13, 2025
* Inlines `scriptingEngineImpl` struct inside `scriptingEngine` struct * Improves log message when engine is already registered Signed-off-by: Ricardo Dias <[email protected]>
…_memory_overhead` It also fixes a bug, where we would never decrement the memory overhead of scripting engines that were unregistered. Signed-off-by: Ricardo Dias <[email protected]>
Signed-off-by: Ricardo Dias <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit creates a new compilation unit for the scripting engine code by extracting the existing code from the functions unit.
We're doing this refactor to prepare the code for running the
EVALcommand using different scripting engines.This PR also fixes #1470.
Signed-off-by: Ricardo Dias [email protected]