Add HSETEX #246
Add HSETEX #246
Conversation
jogehl
requested review from
ahmedsobeh,
bogdanp05,
mkmkme and
smeso
as code owners
mkmkme
left a comment
mkmkme
left a comment
There was a problem hiding this comment.
Thanks for your contribution! Overall it looks great. I added some comments to improve it even further. Could you address them?
| for key, value in mapping.items(): | ||
| pieces.append(key) | ||
| pieces.append(value) | ||
| if items: |
There was a problem hiding this comment.
Probably checking for only one of key + value, mapping or items would be great
| assert r.hsetex("a", mapping=mapping, ex=5) == 1 | ||
| assert r.hget("a", "field1") == b"value1" | ||
| assert r.hget("a", "field2") == b"value2" | ||
|
|
There was a problem hiding this comment.
Could you also add some tests that would check at least EX and PX?
There was a problem hiding this comment.
I have tried this but for this the command HTTL is not implemented yet. Maybe it should be worth to include also this command?
There was a problem hiding this comment.
If you could handle this, I would greatly appreciate it! :)
There was a problem hiding this comment.
If you don't have time for this, you can ignore this for now. I can add the HTTL and add the corresponding tests later
jogehl
force-pushed
the
main
branch
6 times, most recently
from
aa57f75 to
31cf622
Compare
|
Also I've just now realised you didn't provide an async version. Could you add it and the test for it as well? It will be mostly copy-paste |
where can I find the implementations for the async? I can't find the async version for the HashCommands. |
AFAICS AsyncHashCommands is just set to be the same as HashCommands. There is a subtle difference, though. TL;DR: could you add an async test to verify it works? Thanks! |
Signed-off-by: Joshua <joshua.gehlen@fkie.fraunhofer.de>
Signed-off-by: Joshua <joshua.gehlen@fkie.fraunhofer.de>
Signed-off-by: Joshua <joshua.gehlen@fkie.fraunhofer.de>
Signed-off-by: Joshua <joshua.gehlen@fkie.fraunhofer.de>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #246 +/- ##
==========================================
- Coverage 76.28% 76.26% -0.02%
==========================================
Files 129 129
Lines 33906 33992 +86
==========================================
+ Hits 25864 25923 +59
- Misses 8042 8069 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Pull Request check-list
Adds tests and the command HSETEX