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

/topics/lua-api.md has several problems, RESP naming, and other issues. #116

Closed
stockholmux opened this issue May 29, 2024 · 4 comments
Closed

Comments

@stockholmux
Copy link
Member

In pre-publishing review #91, I discovered a number of issues with lua-api.md

FYI: A general problems exist in this document with hash links.

This is a helper function that returns an error reply.

Error reply links to a non-existent hash link /docs/topics/protocol#resp-errors

This is a helper function that returns a simple string reply

simple string reply links to a non-existent hash link /docs/topics/protocol#resp-simple-strings

This function allows the executing script to switch between Valkey Serialization Protocol (RESP) versions

'Valkey Serialization Protocol' is not a thing, should still be Redis Serialization Protocol.

As of Redis OSS version 2.6.0, scripts were replicated verbatim, meaning that the scripts' source code was sent for execution by replicas and stored in the AOF. An alternative replication mode added in version 3.2.0 allows replicating only the scripts' effects. As of Redis OSS version 7.0, script replication is no longer supported, and the only replication mode available is script effects replication.

This should be refactored as 2.6 replication is irrelevant now, and maybe just include 3.2.0 as "before 7". Maybe we don't need to document server.set_repl(x) at all? I'm unclear if it's relevant since there is only effect replication now.

Valkey' replies and Lua's data types and a one-to-one mapping between Lua's data types and the Valkey Protocol data types.

'Valkey Protocol' should be replaced by RESP

* [RESP2 integer reply](protocol.md#resp-integers) -> Lua number
* [RESP2 bulk string reply](protocol.md#resp-bulk-strings) -> Lua string
* [RESP2 array reply](protocol.md#resp-arrays) -> Lua table (may have other Valkey data types nested)
* [RESP2 status reply](protocol.md#resp-simple-strings) -> Lua table with a single _ok_ field containing the status string
* [RESP2 error reply](protocol.md#resp-errors) -> Lua table with a single _err_ field containing the error string

None of these hash links exist on protocol.md

* Lua number -> [RESP2 integer reply](protocol.md#resp-integers) (the number is converted into an integer)
* Lua string -> [RESP bulk string reply](protocol.md#resp-bulk-strings)
* Lua table (indexed, non-associative array) -> [RESP2 array reply](protocol.md#resp-arrays) (truncated at the first Lua nil value encountered in the table, if any)
* Lua table with a single _ok_ field -> [RESP2 status reply](protocol.md#resp-simple-strings)
* Lua table with a single _err_ field -> [RESP2 error reply](protocol.md#resp-errors)

None of these hash links exist on protocol.md

* [RESP3 null](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#null-reply) -> Lua nil.

  • [RESP3 true reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#boolean-reply) -> Lua true boolean value.
  • [RESP3 false reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#boolean-reply) -> Lua false boolean value.
  • [RESP3 double reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#double-type) -> Lua table with a single _double_ field containing a Lua number representing the double value.
  • [RESP3 big number reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#big-number-type) -> Lua table with a single _big_number_ field containing a Lua string representing the big number value.
  • [Valkey verbatim string reply](https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md#verbatim-string-type) -> Lua table with a single _verbatim_string_ field containing a Lua table with two fields, _string_ and _format_, representing the verbatim string and its format, respectively.

None of these hash links exist on https://github.com/redis/redis-specifications/blob/master/protocol/RESP3.md

You can call the SELECT command from your Lua scripts, like you can with any normal client connection. However, one subtle aspect of the behavior changed between Redis OSS versions 2.8.11 and 2.8.12. Prior to Redis OSS version 2.8.12, the database selected by the Lua script was set as the current database for the client connection that had called it. As of Redis OSS version 2.8.12...

Refactor this passage to just put the 2.8.12 way as the way. The rest is irrelevant to Valkey.

@madolson
Copy link
Member

Some of these have been resolved in #85. I need to update that PR.

@zuiderkwast
Copy link
Contributor

The links to protocol.md#xxxxx are fixed in #153.

@zuiderkwast
Copy link
Contributor

I'm deleting all references to older versions like "Since 2.6.0", but to help users compare functionality when they upgrade, we've said that we'll keep documentation for the still maintained versions, i.e. 6.2+.

I'm removing most of the text about verbatim replication, but keeping small parts, because 6.2 is still supported.

Changing "Valkey protocol" to "RESP protocol". (RESP is not an acronym for anything. It's just a name.)

Moving the description of the server object to just above where the fields are documented.

Some formatting and minor changes.

zuiderkwast added a commit to zuiderkwast/valkey-doc that referenced this issue Jul 5, 2024
Signed-off-by: Viktor Söderqvist <[email protected]>
@zuiderkwast
Copy link
Contributor

Closing. This one was fixed in #155 but I forgot to mention it in the top comment. It's mentioned in the commits though.

@stockholmux Do you want to remove the "under review" tag for this page on the website?

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

No branches or pull requests

3 participants