Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding server.call/pcall option to LUA scripting. (#136) #213

Merged
merged 5 commits into from
Apr 6, 2024

Conversation

parthpatel
Copy link
Member

@parthpatel parthpatel commented Apr 4, 2024

This commit does not remove redis.call/pcall just yet. It also does not rename Redis in error messages such as "Please specify at least one argument for this redis lib call". This allows users to maintain full backwards compatibility while introducing an option to use server.call for new code.

I verified that the unit tests pass. Also manually verified that the redis-server responds to server.call invocations within lua scripting. Also verified that function registration works as expected.

[ok]: EVAL - is Lua able to call Redis API? (0 ms)
[ok]: EVAL - is Lua able to call Server API? (1 ms)
[ok]: EVAL - No arguments to redis.call/pcall is considered an error (0 ms)
[ok]: EVAL - No arguments to server.call/pcall is considered an error (1 ms)

src/script_lua.c Outdated Show resolved Hide resolved
src/script_lua.c Outdated Show resolved Hide resolved
tests/unit/scripting.tcl Outdated Show resolved Hide resolved
tests/unit/scripting.tcl Outdated Show resolved Hide resolved
tests/unit/scripting.tcl Outdated Show resolved Hide resolved
src/eval.c Show resolved Hide resolved
This commit does not remove redis.call/pcall just yet. It also does not rename Redis in error messages such as "Please specify at least one argument for this redis lib call". This allows users to maintain full backwards compatibility while introducing an option to use server.call for new code.

Signed-off-by: Parth Patel <661497+parthpatel@users.noreply.github.com>
src/script_lua.c Outdated Show resolved Hide resolved
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Copy link
Member

@madolson madolson left a comment

Choose a reason for hiding this comment

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

The server.register_function are also not working as expected. I'm working on a better way to test all the compatibility atm.

src/eval.c Outdated Show resolved Hide resolved
tests/unit/scripting.tcl Outdated Show resolved Hide resolved
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
tests/unit/scripting.tcl Outdated Show resolved Hide resolved
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
@madolson madolson merged commit 620d325 into valkey-io:unstable Apr 6, 2024
14 checks passed
madolson added a commit to madolson/valkey that referenced this pull request Apr 7, 2024
…lkey-io#213)

This commit does not remove redis.call/pcall just yet. It also does not
rename Redis in error messages such as "Please specify at least one
argument for this redis lib call". This allows users to maintain full
backwards compatibility while introducing an option to use server.call
for new code.

I verified that the unit tests pass. Also manually verified that the
redis-server responds to server.call invocations within lua scripting.
Also verified that function registration works as expected.

```
[ok]: EVAL - is Lua able to call Redis API? (0 ms)
[ok]: EVAL - is Lua able to call Server API? (1 ms)
[ok]: EVAL - No arguments to redis.call/pcall is considered an error (0 ms)
[ok]: EVAL - No arguments to server.call/pcall is considered an error (1 ms)
```

---------

Signed-off-by: Parth Patel <661497+parthpatel@users.noreply.github.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
@zuiderkwast zuiderkwast linked an issue Apr 10, 2024 that may be closed by this pull request
PatrickJS pushed a commit to PatrickJS/placeholderkv that referenced this pull request Apr 24, 2024
…lkey-io#213)

This commit does not remove redis.call/pcall just yet. It also does not
rename Redis in error messages such as "Please specify at least one
argument for this redis lib call". This allows users to maintain full
backwards compatibility while introducing an option to use server.call
for new code.

I verified that the unit tests pass. Also manually verified that the
redis-server responds to server.call invocations within lua scripting.
Also verified that function registration works as expected.

```
[ok]: EVAL - is Lua able to call Redis API? (0 ms)
[ok]: EVAL - is Lua able to call Server API? (1 ms)
[ok]: EVAL - No arguments to redis.call/pcall is considered an error (0 ms)
[ok]: EVAL - No arguments to server.call/pcall is considered an error (1 ms)
```

---------

Signed-off-by: Parth Patel <661497+parthpatel@users.noreply.github.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rebranding Valkey is not Redis
Projects
Development

Successfully merging this pull request may close these issues.

Add Lua global object server as alias for redis
4 participants