Moving to tox-docker

[WisdomPill]

Fixes #574

This PR is to make tox manage docker as redis-py is doing.

I still have some issues with sentinel, I seem to not understand how it should work.

@chayim could you help me understand where I got the configuration wrong?
Also another couple of questions, why you are not using redislab docker hub repository?
What is the difference?

@terencehonles maybe you also have some hints?

After that the unix socket will be the last problem to solve, but for now I think it is not very important

[codecov]

Codecov Report

Merging #583 (3c4e363) into master (4e7f2d4) will decrease coverage by 23.9%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #583      +/-   ##
=========================================
- Coverage    57.8%   33.9%   -23.8%     
=========================================
  Files          39      39              
  Lines        2497    2465      -32     
  Branches       73      73              
=========================================
- Hits         1441     834     -607     
- Misses       1041    1558     +517     
- Partials       15      73      +58     

Flag	Coverage Δ
mypy	33.9% <ø> (ø)
tests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
django_redis/client/sharded.py	5.9% <0.0%> (-80.1%)	⬇️
django_redis/pool.py	11.0% <0.0%> (-66.0%)	⬇️
django_redis/cache.py	34.2% <0.0%> (-62.8%)	⬇️
django_redis/serializers/msgpack.py	37.5% <0.0%> (-62.5%)	⬇️
django_redis/client/sentinel.py	22.8% <0.0%> (-54.5%)	⬇️
django_redis/client/default.py	38.8% <0.0%> (-51.6%)	⬇️
django_redis/serializers/base.py	50.0% <0.0%> (-50.0%)	⬇️
django_redis/serializers/pickle.py	54.6% <0.0%> (-45.4%)	⬇️
django_redis/serializers/json.py	60.0% <0.0%> (-40.0%)	⬇️
django_redis/compressors/zstd.py	61.6% <0.0%> (-38.4%)	⬇️
... and 7 more

---

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 4e7f2d4...3c4e363. Read the comment docs.

[chayim]

@WisdomPill You may want to think about getting these out of your github actions and into something like a dev_dependencies (or pyproject.toml dev area). This way they're not as hidden from users.

[WisdomPill]

Yes, I guess I will add some kind of dev script to setup the environment.

I still do not really understand how tox installs dependencies and where is it getting redislatest.

These are my first steps with tox

[chayim]

Got it. Here's how I think about tox with docker:

• Define a series of docker images that may or may not get used
• Define tox environments that depend on individual dockers
• Add pre_commands or dependencies if/as necessary
• Run tests

For one project I use this pattern to parameterize a pytest option, so that we can control test execution and dockers being run. It may be a better example for your pattern.

[chayim]

@WisdomPill Awesome to see you also thinking of this approach! We ended up spinning our own dockers and pushing them to dockerhub, because it was the easiest way for us to proceed - while still having everything we need. Maybe that workflow is useful, or maybe those dockers are useful?

I'm planning on (future) adding support for multiple versions of redis as well, so maybe we an collaborate there? If it helps any, I'm happy to push to dockerfab for you. Also - let me know how I can help!

[WisdomPill]

Now the problem seems to be that sentinel does find the master node, or that it is disconnected somehow.

Do you know if the configuration is wrong?
The previous configuration scripts are under test/start-redis.sh and test/wait_for_redis.sh.
They were setup in the this manner.
This is my first experience with sentinel, usually it is @terencehonles who is in charge of it.

I would love to keep contributing maybe even with some python code to redis-py, count me in for testing multiple versions of redis, this is something we are also looking for.

After solving this issue I would like also to tackle the usage of original docker images from redis.

[chayim]

Do you know if the configuration is wrong?

Our approach is a bit different. I pushed a series of dedicated dockers that I maintain and built once, from the community redis docker to dockerhub. Those dockers each load a configuration file from disk. You'll note the base docker folder, and the subsequent configuration files for a sentinel node, or a replica.

Extending that we have tox-docker run a healthcheck for each docker, ensuring they're at least up. By controlling redis from outside, with the configuration file it becomes easy to (more) rapidly iterate on particular configuration needs.

This was the reason I built the base docker from the community docker image, rather than our redis enteprise docker. Using that docker, and loading a configuration file that is mounted into the container using the volume mounts, added the flexibility needed for environments.

Hope this helps @WisdomPill

[WisdomPill]

@chayim thanks for the great explanation.

But, my problem is not on how to use tox-docker, my problem is with sentinel, it seems like the sentinel does not connect to the redis master, so my question is, is there something wrong with the configuration I have made? Could you have a look at it?

I copied the configuration from redis-py and tried to make it work, but for a normal instance is working, for sentinel not.

[chayim]

@WisdomPill Can you help walking me through running this locally on a single python? Looking at the dockers you're starting via tests/start_redis.sh the sentinel definitely sees the master - at least locally. I wasn't able to get pytest in my world functioning and running your tests - which I'm sure is on my side.

But, I validated connectivity by running (locally, outside of the dockers)

# connecting to the master, yes it's up
redis-cli -p 6379
PING # returns an ok

Now, I connected to the sentinel node

# to connect to the sentinel node
redis-cli -p 26379
PING # returns an ok

# to show me sentinel settings (since I see master0:name=default_service) I'm sure it is configured to see the master
INFO

# allows me to validate that yes, it is pointed to a master
SENTINEL get-master-addr-by-name default_service

# show me information from sentinel about the master
SENTINEL MASTER default_service

With the above INFO command, in the sentinel context you'll see last-ok-ping-reply and it constantly changes, meaning there is connectivity. Similarly, running the redis-cli from within the individual dockers helped me validate connectivity another way.

I'm no sentinel expert, but I think you need at least 3 nodes (so one more, see the text "Example 1: just two Sentinels, DON'T DO THIS). Because SENTINEL CKQUORUM default_service returns NOQUORUM 1 usable Sentinels. Not enough available Sentinels to reach the specified quorum for this master. Similarly, the other sentinel help commands SENTINEL replicas default_service and SENTINEL sentinels default_service both return empty arrays.

Hope this helps - and happy to debug more if you can help me on the environment side!

[chayim]

@WisdomPill pinging you here too, as I'm happy to work on this in tandem.

[WisdomPill]

I did not have time to work on open source lately, I will get back to you soon