RedisJSON / redisjson-py

An extension to redis-py for using Redis' ReJSON module
https://redisjson.io
BSD 2-Clause "Simplified" License
160 stars 34 forks source link

#39 - Let's discuss the approach and tests #40

Closed radoye closed 10 months ago

radoye commented 4 years ago

Here's a quick implementation, without going into all the details of redis-py/redis-py-cluster.

Before spending more time on this, it would be useful to hear comments on

  1. tests
  2. style/approach.

Tests

Redis

On all my systems, a single failure happens both after and before my edits. Can you confirm/comment?

Singularity> python -m unittest tests/test_rejson.py 
........F...........
======================================================================
FAIL: testJSONSetGetDelNonAsciiShouldSucceed (tests.test_rejson.ReJSONTestCase)
Test non-ascii JSONSet/Get/Del
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rrs/docks/dev/numericcal/os/redisjson-py/tests/test_rejson.py", line 46, in testJSONSetGetDelNonAsciiShouldSucceed
    self.assertEqual('hyvää-élève', rj.jsonget('notascii'))
AssertionError: 'hyvää-élève' != 'hyvää-élève'
- hyvää-élève
+ hyvää-élève

----------------------------------------------------------------------
Ran 20 tests in 0.017s

FAILED (failures=1)

Redis Cluster

The same failure and a few errors. I'm not sure that these should pass for the cluster. Can you comment?

Singularity> export REDIS_HOST="localhost"                                                           
Singularity> export REDIS_PORT="6379"                                                                
Singularity> python -m unittest tests/test_rejson.py                                                 
........F.E....E...E                                                                                 
======================================================================                               
ERROR: testMGetShouldSucceed (tests.test_rejson.ReJSONTestCase)                                      
Test JSONMGet                                                                                        
----------------------------------------------------------------------                               
Traceback (most recent call last):                                                                   
  File "/home/rrs/workspace/docks/dev/numericcal/DataKube/gh_deps/redisjson-py/tests/test_rejson.py",
 line 74, in testMGetShouldSucceed                                                                   
    r = rj.jsonmget(Path.rootPath(), '1', '2')                                                       
  File "/home/rrs/workspace/docks/dev/numericcal/DataKube/gh_deps/redisjson-py/rejson/client.py", lin
e 177, in jsonmget                                                                                   
    return self.execute_command('JSON.MGET', *pieces)                                                
  File "/home/rrs/workspace/docks/dev/numericcal/DataKube/gh_deps/redisjson-py/rejson/client.py", lin
e 85, in execute_command                                                                             
    return self.client.execute_command(*args, **kwargs)                                              
  File "/home/rrs/.local/lib/python3.7/site-packages/rediscluster/utils.py", line 101, in inner      
    return func(*args, **kwargs)                                                                     
  File "/home/rrs/.local/lib/python3.7/site-packages/rediscluster/client.py", line 410, in execute_co
mmand                                                                                                
    return self.parse_response(r, command, **kwargs)                                                 
  File "/home/rrs/.local/lib/python3.7/site-packages/redis/client.py", line 768, in parse_response   
    response = connection.read_response()
  File "/home/rrs/.local/lib/python3.7/site-packages/redis/connection.py", line 638, in read_respons$
    raise response
rediscluster.exceptions.ClusterCrossSlotError: Keys in request don't hash to the same slot

======================================================================
ERROR: testPipelineShouldSucceed (tests.test_rejson.ReJSONTestCase)
Test pipeline
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rrs/workspace/docks/dev/numericcal/DataKube/gh_deps/redisjson-py/tests/test_rejson.py"$ line 187, in testPipelineShouldSucceed
    self.assertListEqual([True, 'bar', 1, False], p.execute())
  File "/home/rrs/.local/lib/python3.7/site-packages/redis/client.py", line 3437, in execute
    self.shard_hint)
  File "/home/rrs/.local/lib/python3.7/site-packages/rediscluster/connection.py", line 196, in get_co
nnection
    raise RedisClusterException("Only 'pubsub' commands can be used by get_connection()")
rediscluster.exceptions.RedisClusterException: Only 'pubsub' commands can be used by get_connection()

======================================================================
ERROR: testUsageExampleShouldSucceed (tests.test_rejson.ReJSONTestCase)
Test the usage example
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rrs/workspace/docks/dev/numericcal/DataKube/gh_deps/redisjson-py/tests/test_rejson.py",
 line 270, in testUsageExampleShouldSucceed
    jp.execute()
  File "/home/rrs/.local/lib/python3.7/site-packages/redis/client.py", line 3437, in execute
    self.shard_hint)
  File "/home/rrs/.local/lib/python3.7/site-packages/rediscluster/connection.py", line 196, in get_co
nnection
    raise RedisClusterException("Only 'pubsub' commands can be used by get_connection()")
rediscluster.exceptions.RedisClusterException: Only 'pubsub' commands can be used by get_connection()

======================================================================
FAIL: testJSONSetGetDelNonAsciiShouldSucceed (tests.test_rejson.ReJSONTestCase)
Test non-ascii JSONSet/Get/Del
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rrs/workspace/docks/dev/numericcal/DataKube/gh_deps/redisjson-py/tests/test_rejson.py",
 line 46, in testJSONSetGetDelNonAsciiShouldSucceed
    self.assertEqual('hyvää-élève', rj.jsonget('notascii'))
AssertionError: 'hyvää-élève' != 'hyvää-élève'
- hyvää-élève
+ hyvää-élève

----------------------------------------------------------------------
Ran 20 tests in 0.827s

FAILED (failures=1, errors=3)
Singularity>
lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts and fixes 1 when merging 0b9845f017f636a9909d6d214d6a8370bf5f9bf9 into 4cf10492a7e902f3662b7e6adea4f383471cb48c - view on LGTM.com

new alerts:

fixed alerts:

codecov[bot] commented 4 years ago

Codecov Report

Merging #40 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #40   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          114       114           
=========================================
  Hits           114       114           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4cf1049...0b9845f. Read the comment docs.