-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add RESP2 to RESP3 replies migration guide #2513
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
base: master
Are you sure you want to change the base?
Conversation
👷 Deploy request for redis-doc pending review.Visit the deploys page to approve it
|
I believe there may be some more "verbatim string" swaps; |
To @mgravell's point - |
@mgravell @slorello89 @oranagra Sorry for the delay. The document was updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a pretty thorough review of the RESP3 v RESP2 command results for OSS on my own - this list seems comprehensive to me.
Overall changes: - [x] introduce `Resp2Type` and `Resp3Type` shims (`Resp2Type` has reduced types); existing code using `[Obsolete] Type` uses `Resp2Type` for minimal code impact - [x] mark existing `Type` as `[Obsolete]`, and proxy to `Resp2Type` for compat - [x] deal with null handling differences - [x] deal with `Boolean`, which works very differently (`t`/`f` instead of `1`/`0`) - [x] deal with `[+|-]{inf|nan}` when parsing doubles (explicitly called out in the RESP3 spec) - [x] parse new tokens - [x] `HELLO` handshake - [x] core message and result handling - [x] validation and fallback - [x] prove all return types (see: redis/redis-specifications#15) - [x] <strike>streamed RESP3</strike> omitting; not implemented by server; can revisit - [x] deal with pub/sub differences - [x] check routing of publish etc - [x] check re-wire of subscription if failed - [x] check receive notifications - [x] connection management (i.e. not to spin up in resp3) - [x] connection fallback spinup (i.e. if we were trying resp3 but failed) - [x] other - [x] [undocumented RESP3 delta](redis/redis-doc#2513) - [x] run core tests in both RESP2 and RESP3 - [x] compensate for tests that expect separate subscription connections Co-authored-by: Nick Craver <[email protected]> Co-authored-by: Nick Craver <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some copy-edits and style guide updates.
I also suggest using the same type names that are found on the protocol page. For example, change "[bB]lob[sS]tring" to "bulk string" (?) as appropriate.
Co-authored-by: David Dougherty <[email protected]>
just wondering... any plans to merge this ever? |
See #2511 for more details.