connections: fix pipeline usage for exists command

[utkarshgupta137]

Potential fix for: redis/redis-py#2407

[codecov]

Codecov Report

Merging #366 (5794784) into main (9ce2fd6) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #366   +/-   ##
=======================================
  Coverage   98.80%   98.80%           
=======================================
  Files           9        9           
  Lines         917      917           
  Branches      166      166           
=======================================
  Hits          906      906           
  Misses          6        6           
  Partials        5        5           

Impacted Files	Coverage Δ
arq/connections.py	95.83% <100.00%> (ø)

---

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 9ce2fd6...5794784. Read the comment docs.

[chayim]

@utkarshgupta137 Don't know if you saw the python 3.7 (redis 5) CI fail here. Tagged you because...

[utkarshgupta137]

@utkarshgupta137 Don't know if you saw the python 3.7 (redis 5) CI fail here. Tagged you because...

I saw the failure, but I'm unable to figure out the issue from a brief look. I don't have the time to look into it, so I figured that the library's maintainers would be able to check why it is failing.

[samuelcolvin]

I think there are some intermittent failures due to timing issues, I've rerun the job in the hope it passes.

[samuelcolvin]

Please can someone explain why this can't be run as part of the pipeline? It would surely be cleaner and faster to include this in the pipeline.

[utkarshgupta137]

Please can someone explain why this can't be run as part of the pipeline? It would surely be cleaner and faster to include this in the pipeline.

You can run it in pipeline. The problem is that you're calling the same pipeline twice in parallel (asyncio.gather). You should either call the pipeline once or not use pipeline if calling in parallel.

[samuelcolvin]

makes sense, then I would suggest we drop the asyncio.gather do the queries sequentially but using the pipeline.

[utkarshgupta137]

makes sense, then I would suggest we drop the asyncio.gather do the queries sequentially but using the pipeline.

Even better: Since the exists command can accept multiple keys, we can check both of them in a single command. Not sure if you still need a pipeline with that, but I've left it since you're using a transaction.

[JonasKs]

Pipeline does not test against release candidate on redis-py, have you tried locally and ensure this works? If not I can check later / we should add a pipeline.

EDIT: I just tested, it works on redis==4.4.0rc4

[samuelcolvin]

Thanks so much.

Is anything else required to support redis==4.4.0?

[JonasKs]

Installed arq from master in one of my projects and all tests + functionality seems to run fine here now. Thanks a lot all of you 😊