Skip to content
This repository has been archived by the owner on Jan 9, 2024. It is now read-only.

[WIP] Feat/watch enhancement #251

Closed
wants to merge 5 commits into from
Closed

[WIP] Feat/watch enhancement #251

wants to merge 5 commits into from

Conversation

alwqx
Copy link

@alwqx alwqx commented Apr 7, 2018

Hi @Grokzen #201 seems not maintain, I need watch too, so I make this pr based @max8899 code.

max8899 and others added 5 commits April 7, 2018 17:28
Use transaction with pipeline in redis cluster

Do the operation with keys with same prefix, so that command will be
executed on the same node

* pipeline

Signed-off-by: Lei Gong <xue177125184@gmail.com>
Use transaction with pipeline in redis cluster

Do the operation with keys with same prefix, so that command will be
executed on the same node

* Reference pipeline function

Signed-off-by: Lei Gong <xue177125184@gmail.com>
@alwqx
Copy link
Author

alwqx commented Apr 9, 2018

Hi @Grokzen ,I run tests with command pytest --cov rediscluster -k test_pipeline_no_transaction_watch in my local develop environment. But got below error:

________________________________________ TestPipeline.test_pipeline_no_transaction_watch _________________________________________

self = <tests.test_pipeline.TestPipeline object at 0x104947510>, r = StrictRedisCluster<127.0.0.1:7000>

    def test_pipeline_no_transaction_watch(self, r):
        r['a'] = 0

        with r.pipeline(transaction=False) as pipe:
            pipe.watch('a')
            a = pipe.get('a')

>           pipe.multi()

tests/test_pipeline.py:78:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = StrictClusterPipeline, names = ()

    def multi(self, *names):
        """
            """
        if not names:
>           raise RedisClusterException("MULTI command needs at least one name in this pipeline")
E           RedisClusterException: MULTI command needs at least one name in this pipeline

rediscluster/pipeline.py:251: RedisClusterException

It seems the MULTI method not implemented well, can you give some help about multi method. otherwise I can not pass tests.

@Grokzen
Copy link
Owner

Grokzen commented Apr 9, 2018

@adolphlwq You have implemented the def multi() function wrong as it should be mirrored the normal redis client like this here def multi(self): https://github.com/andymccurdy/redis-py/blob/master/redis/client.py#L2683

@Grokzen
Copy link
Owner

Grokzen commented Jun 2, 2018

@adolphiwq Any update on this MR? No progress for a while not.

@alwqx
Copy link
Author

alwqx commented Jun 4, 2018

multi method mirrored from redis client is not compatibility for cluster mode.
Also, I find it difficult to implement multi and transaction in redis cluster mode,
Will work on it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants