-
Notifications
You must be signed in to change notification settings - Fork 17
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
i40: Support for Redis 6.2.0 #41
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #41 +/- ##
===========================================
- Coverage 100.00% 99.39% -0.61%
===========================================
Files 18 18
Lines 1476 1642 +166
===========================================
+ Hits 1476 1632 +156
- Misses 0 10 +10
Continue to review full report at Codecov.
|
IIUC, are you suggesting that there might be a set of subcommands not normally available in r <- hiredis()
mygrp <- r$consumer_group("mystream", "mygroup", "myname")
mygrp$XADD(...)
mygrp$XREAD(...) (Perhaps mangled a bit.) I don't like that interface, to be honest. I think the most straight-forward path for My vote: add |
+1 that the streaming features should be exposed at the level consistent with existing functions, and sugar can be built on top of them later, either in this package (with a different naming scheme ideally) or elsewhere. |
Thanks @kevinykuo and @r2evans for the feedback. I will pick this back up as soon as I have time again for personal open source work (I've been knee-deep in covid work since mid-November with no spare time) |
- regenerate API to support redis 5.0 (redis doc commit db399bb9f3871a749bc14cc9d60761a7bfd73cf8) - add .o. .so to gitignore
8ba428e
to
6c7b4e3
Compare
Codecov Report
@@ Coverage Diff @@
## master #41 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 18 18
Lines 1479 1722 +243
==========================================
+ Hits 1479 1722 +243
Continue to review full report at Codecov.
|
Support up to ~6.0.0 is ok, but after that we get a new block argument type that I need to handle: |
@richfitz, I recognize that |
Adds support for new commands added through to Redis 6.2.0, incorporating changes in #28, fixing failing tests and adding tests for the new functions.
The interface to the stream (
X*
) functions I am not immediately happy with, as things likeXREADGROUP
I am not sure are properly supported yet, and commands likeXGROUP
really feel like they should be split into subcommands. It might be easiest to fold this in for now and then fix, but to do this properly I probably will have to either understand the streaming interface or get input from someone who has used it (@dseynaev perhaps?).Fixes #40