Support connection schemes valkey:// and valkeys:// by zhulipeng · Pull Request #199 · valkey-io/valkey

zhulipeng · 2024-04-04T10:33:16Z

Support valkey:// and valkeys:// scheme in valkey-cli and valkey-benchmark. Retain the original Redis schemes for compatibility.
Add unit tests for valid URI, all schemes.

Fixes: #198
Fixes: #200

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

zuiderkwast

LGTM.

The --help text now only mentions the valkey schemes, although the redis schemes also still work.

@valkey-io/core-team Do we need to document in the help text that the redis schemes are still supported?

zuiderkwast · 2024-04-04T12:40:25Z

AH!! I didn't review the tests. If there is more coming, better mark the PR as draft. :D

I'll review the tests now.

zuiderkwast

OK, LGTM, just one typo in one of the test case names. :)

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

enjoy-binbin · 2024-04-04T13:34:00Z

Please also update the top comment to describe the changes (don't just link issue).
For example, mention we supports valkey://, and also retains the original redis schema for compatibility, and then we adds some tests to cover these cases.

also if you have a PR directly, there is actually no need to create a new issue (there is no discussion or tracking in the issue so it is actually worthless)

madolson · 2024-04-04T15:48:24Z

-    const char *scheme = "redis://";
-    const char *tlsscheme = "rediss://";
+    const char *scheme = "valkey://";
+    const char *tlsscheme = "valkeys://";


I have a concerns about this. With Redis, using rediss was clearer to me that it was SSL because there is obviously a second s. With Valkey, I'm concerned valkeys will be confusing since it's not clear. I want us to least thing about it a bit more.

@madolson think about it for a while then. :)

Adding "s" is the most common style for URI schemes, so I think valkeys is the right choice. Here are some from IANA's list:

aaa Diameter Protocol aaas Diameter Protocol with Secure Transport http Hypertext Transfer Protocol https Hypertext Transfer Protocol Secure msrp Message Session Relay Protocol msrps Message Session Relay Protocol Secure rtsp Real-Time Streaming Protocol (RTSP) rtsps Real-Time Streaming Protocol (RTSP) over TLS sip session initiation protocol sips secure session initiation protocol

Ok, I thought about it and will retract my concern :) I still don't really like it, but I guess it's fine.

I guess now would be the time to rethink the URI scheme if we wanted to do that though.

I don't know if this scheme is widely used. It's a client feature anyway.

We can postpone it indefinitely and only support the redis scheme. Do you prefer that?

We can consider resp as the scheme. RESP is the protocol after all. :)

I like resp more. In fact, I think "redis" was a minomer to begin with. It should've been a protocol name.

I think the redis scheme contains more than what's specified in RESP.

An example URL redis://user:password@host:port/dbnum contains information for the AUTH command and the SELECT command, to be called after connecting. It doesn't contain the RESP version though (for HELLO). It's a bit ad-hoc all of it.

It could have included various more things which are useful to do when connecting, like CLIENT SETNAME, CLIENT SETINFO LIBNAME, CLIENT SETINFO LIBVER, CLIENT NO-EVICT.

For example we could support them in a query string, such as

?name=banana&libname=valkey-py&libver=3.14.159&no-evict=1

The RESP version (HELLO N) very much affects what comes out from each command, so it's basically a different protocol. We could embed that in the scheme as resp2: vs resp3:.

.... or we can just keep the redis schemes just for backward compatibility. I'm not sure we need a scheme anyway, do we?

As long as valkey-cli can accept "redis/rediss", I am fine with other options, "valkey" or "resp2/3".

@zuiderkwast, you have a good point that the scheme implies more than just the wire protocol but also the specific client behavior so this is more of a call on the client side.

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

zuiderkwast · 2024-04-23T01:01:09Z

I'm going to merge this. It's the least surprising schemes for any forked clients and for users moving from redis-cli to valkey-cli.

We can change back if there are protests.

1. Support valkey:// and valkeys:// scheme in valkey-cli and valkey-benchmark. Retain the original Redis schemes for compatibility. 2. Add unit tests for valid URI, all schemes. Fixes: valkey-io#198 Fixes: valkey-io#200 --------- Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

) When using a TLS scheme for valkey-cli and benchmark-cli compiled without TLS, make the error message only mention the scheme used. Before: "valkeys:// and rediss:// are only supported when valkey-cli is compiled with OpenSSL" After, depending on which scheme the user tried to use: "valkeys:// is only supported when valkey-cli is compiled with OpenSSL" "rediss:// is only supported when valkey-cli is compiled with OpenSSL" Follow up of #199. --------- Signed-off-by: karthyuom <karthyuom@gmail.com> Co-authored-by: k00809413 <karthick.ariyaratnam1@huawei.com>

Squashed 'deps/libvalkey/' changes from abcd27fbf..d95a5ee9a 9e10acbf7 Fix duplicate Acks for RDMA events. (valkey-io#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (valkey-io#205) 178e350c7 Cluster code cleanup (valkey-io#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (valkey-io#212) 279125e22 Spelling (valkey-io#213) 99aa158bc Use existing connections for blocking slotmap updates (valkey-io#199) f670346b9 Only install headers for `TLS` and `RDMA` when enabled in CMake (valkey-io#206) 41c5911f1 Remove macro UNUSED from public API (valkey-io#200) 969a8c546 Fix dependency issue with RDMA (valkey-io#201) git-subtree-dir: deps/libvalkey git-subtree-split: d95a5ee9ad95b9ee7f298f38d0fb93f3f7659ef7 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

Squashed 'deps/libvalkey/' changes from abcd27fbf..b012f8e85 b012f8e85 Release 0.2.1 9e10acbf7 Fix duplicate Acks for RDMA events. (valkey-io#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (valkey-io#205) 178e350c7 Cluster code cleanup (valkey-io#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (valkey-io#212) 99aa158bc Use existing connections for blocking slotmap updates (valkey-io#199) 969a8c546 Fix dependency issue with RDMA (valkey-io#201) git-subtree-dir: deps/libvalkey git-subtree-split: b012f8e85a9cc2c68bc2be982a4f6545c15042f0 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

Squashed 'deps/libvalkey/' changes from abcd27fbf..b012f8e85 b012f8e85 Release 0.2.1 9e10acbf7 Fix duplicate Acks for RDMA events. (valkey-io#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (valkey-io#205) 178e350c7 Cluster code cleanup (valkey-io#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (valkey-io#212) 99aa158bc Use existing connections for blocking slotmap updates (valkey-io#199) 969a8c546 Fix dependency issue with RDMA (valkey-io#201) ... git-subtree-dir: deps/libvalkey git-subtree-split: b012f8e85a9cc2c68bc2be982a4f6545c15042f0 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

Squashed 'deps/libvalkey/' changes from abcd27fbf..b012f8e85 b012f8e85 Release 0.2.1 9e10acbf7 Fix duplicate Acks for RDMA events. (#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (#205) 178e350c7 Cluster code cleanup (#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (#212) 99aa158bc Use existing connections for blocking slotmap updates (#199) 969a8c546 Fix dependency issue with RDMA (#201) ... git-subtree-dir: deps/libvalkey git-subtree-split: b012f8e85a9cc2c68bc2be982a4f6545c15042f0 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

Squashed 'deps/libvalkey/' changes from abcd27fbf..b012f8e85 b012f8e85 Release 0.2.1 9e10acbf7 Fix duplicate Acks for RDMA events. (valkey-io#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (valkey-io#205) 178e350c7 Cluster code cleanup (valkey-io#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (valkey-io#212) 99aa158bc Use existing connections for blocking slotmap updates (valkey-io#199) 969a8c546 Fix dependency issue with RDMA (valkey-io#201) ... git-subtree-dir: deps/libvalkey git-subtree-split: b012f8e85a9cc2c68bc2be982a4f6545c15042f0 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

Squashed 'deps/libvalkey/' changes from abcd27fbf..b012f8e85 b012f8e85 Release 0.2.1 9e10acbf7 Fix duplicate Acks for RDMA events. (#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (#205) 178e350c7 Cluster code cleanup (#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (#212) 99aa158bc Use existing connections for blocking slotmap updates (#199) 969a8c546 Fix dependency issue with RDMA (#201) ... git-subtree-dir: deps/libvalkey git-subtree-split: b012f8e85a9cc2c68bc2be982a4f6545c15042f0 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech>

Squashed 'deps/libvalkey/' changes from abcd27fbf..b012f8e85 b012f8e85 Release 0.2.1 9e10acbf7 Fix duplicate Acks for RDMA events. (valkey-io#229) 1eadedf48 Remove the unused valDup API from dict a449f0ea1 Don't expose internal functions in shared libraries (valkey-io#205) 178e350c7 Cluster code cleanup (valkey-io#216) 4020396c8 Fix `unused-parameter` warning when building with `NDEBUG` (valkey-io#212) 99aa158bc Use existing connections for blocking slotmap updates (valkey-io#199) 969a8c546 Fix dependency issue with RDMA (valkey-io#201) ... git-subtree-dir: deps/libvalkey git-subtree-split: b012f8e85a9cc2c68bc2be982a4f6545c15042f0 Signed-off-by: Björn Svensson <bjorn.a.svensson@est.tech> Signed-off-by: Harkrishn Patro <harkrisp@amazon.com>

roshkhatri · 2026-01-30T02:15:33Z

@zuiderkwast Need to back port this to 7.2 too

zuiderkwast · 2026-01-30T11:14:17Z

Isn't it a new feature? We're not supposed to add new features in a patch version.

... or can it be assumed that this is implicitly part of the rebranding?

rubiin approved these changes Apr 4, 2024

View reviewed changes

zuiderkwast reviewed Apr 4, 2024

View reviewed changes

Comment thread src/cli_common.c

Rename connection scheme from redis:// to valkey://.

244df8e

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

zhulipeng changed the title ~~Rename connection scheme from redis:// to valkey://.~~ Support connection schemes: valkey://, valkeys://. Apr 4, 2024

zhulipeng changed the title ~~Support connection schemes: valkey://, valkeys://.~~ Support connection schemes: valkey://, valkeys://. Apr 4, 2024

zhulipeng force-pushed the scheme branch from d21120f to 63a27aa Compare April 4, 2024 12:26

zuiderkwast approved these changes Apr 4, 2024

View reviewed changes

Gemrav mentioned this pull request Apr 4, 2024

Fixes: https://github.com/valkey-io/valkey/issues/198 tcurdt/jdeb#650

Closed

zuiderkwast approved these changes Apr 4, 2024

View reviewed changes

Comment thread tests/integration/redis-cli.tcl Outdated

zhulipeng added 3 commits April 4, 2024 08:49

Support redis:// and rediss:// scheme for compatibility.

63a27aa

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

Add unit tests for valid connection scheme.

0864507

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

Fix typo.

d789b5b

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

enjoy-binbin reviewed Apr 4, 2024

View reviewed changes

Comment thread tests/integration/redis-cli.tcl Outdated

madolson reviewed Apr 4, 2024

View reviewed changes

Resolve conflicts with unstable.

9a09228

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>

This was referenced Apr 12, 2024

Rename connection scheme from redis:// to valkey:// . #198

Closed

Replace "redis" with "valkey" test code #287

Merged

zuiderkwast merged commit 87a5bfc into valkey-io:unstable Apr 23, 2024

zuiderkwast changed the title ~~Support connection schemes: valkey://, valkeys://.~~ Support connection schemes valkey:// and valkeys:// Apr 23, 2024

zuiderkwast added the release-notes This issue should get a line item in the release notes label Apr 23, 2024

zhulipeng deleted the scheme branch April 23, 2024 02:59

karthyuom mentioned this pull request Apr 25, 2024

Mention one of rediss:// and valkeys:// in error message, not both #373

Merged

displexic mentioned this pull request Mar 5, 2025

Add Valkey URL scheme redis-rs/redis-rs#1558

Merged

Uh oh!

Conversation

zhulipeng commented Apr 4, 2024 • edited by zuiderkwast Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

zuiderkwast commented Apr 4, 2024

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

enjoy-binbin commented Apr 4, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madolson Apr 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuiderkwast commented Apr 23, 2024

Uh oh!

roshkhatri commented Jan 30, 2026

Uh oh!

zuiderkwast commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zhulipeng commented Apr 4, 2024 •

edited by zuiderkwast

Loading

madolson Apr 4, 2024 •

edited

Loading