Make hset support multiple field/value pairs.#1271
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1271 +/- ##
==========================================
- Coverage 92.39% 91.13% -1.27%
==========================================
Files 19 19
Lines 6394 6249 -145
==========================================
- Hits 5908 5695 -213
- Misses 486 554 +68
Continue to review full report at Codecov.
|
|
Thanks for making this PR. I'd prefer to not use keyword args here -- this was a big mistake with ZADD from the 2.x days and 3.x ended up breaking a bunch of user code. Instead, lets make it look like:
Also we should adjust the comment so that it mentions that the return value is the number of fields added to the hash (as the redis docs suggest). |
8a76f68 to
77408cb
Compare
|
@andymccurdy Hi thanks for review, updated. PTAL again :) |
|
ping @andymccurdy |
cda787a to
ddbc77a
Compare
Co-Authored-By: Alan Mai <0110amai@gmail.com>
|
Thanks, looks great! |
|
I like this commit and it is such an important interface. |
|
We could consider a backwards incompatible change when we increment a major version number. For example, 3.x -> 4.x. |
| Set ``key`` to ``value`` within hash ``name`` | ||
| Returns 1 if HSET created a new field, otherwise 0 | ||
| Set ``key`` to ``value`` within hash ``name``, | ||
| Use ``mappings`` keyword args to set multiple key/value pairs |
There was a problem hiding this comment.
Small typo here: s/mappings/mapping
There was a problem hiding this comment.
Thanks, but it's already fixed in master: https://github.com/andymccurdy/redis-py/blob/master/redis/client.py#L3037
| * HSET command now can accept multiple pairs. HMSET has been marked as | ||
| deprecated now. Thanks to @laixintao #1271 |
There was a problem hiding this comment.
@laixintao this unfortunately doesn't mention that it requires at least Redis 4.0.0. We got bitten by this when updating redis-py (from 3.4.1 to 3.5.2) in a system that runs Redis 3.2.4.
@andymccurdy I couldn't find any information about which version(s) of Redis is supported by redis-py. Did I miss something?
There was a problem hiding this comment.
@ala-ableton This is completely backwards compatible with older versions of Redis server. You can still use the key and value that were present in all previous versions of redis-py. We could add a comment to the function that mapping requires Redis server 4.0 or greater.
redis-py works with all known versions of Redis server. Obviously you need a version of Redis server that supports the commands and command options you're working with.
There was a problem hiding this comment.
@andymccurdy sorry, I misread the diff. It seems that our issue didn't come from redis-py, but from https://github.com/rq/rq (more specifically, rq/rq@5859339). Thanks for the quick reply!
Pull Request check-list
Please make sure to review and check all of these items:
$ toxpass with this change (including linting)?NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
hsetaccept multiple field/value s now.hmsetmarked as deprecated.close #1269