#39 - Let's discuss the approach and tests

[radoye]

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]

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

new alerts:

• 1 for Conflicting attributes in base classes
• 1 for Module is imported with 'import' and 'import from'

fixed alerts:

• 1 for Missing call to __init__ during object initialization

[codecov]

Codecov Report

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

@@            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.

[MeirShpilraien]

@radoye why not creating a decorator for all the client functions?

[radoye]

Sure. I'm happy to clean up once it's ready for merging.

1. Let's figure out if tests make sense, or I overlooked something.
2. Except for hiding these trivial functions with a decorator, any other approach comments?