Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Migrator throws Moved error when connecting to cluster #550

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

a9raag
Copy link

@a9raag a9raag commented Aug 3, 2023

While connecting to a Redis Cluster Migrator run fails to create_index since the commands are executed using a Redis connection.
In this scenario, there should be some way to distinguish between Redis and RedisCluster in order to avoid:

redis.exceptions.MovedError: 13099 as mentioned in #408 #412
With RedisCluster support there would be an option to execute these commands on primary nodes and we won't run into the MovedError

In this PR:
We can connect with RedisCluster using two ways:

  • I have updated get_redis_connection method to allow cluster parameter so it would be as straight forward as using get_redis_connection(host='localhost', cluster=True)
  • By providing cluster=True in URL, for example get_redis_connection(url="redis://localhost:6379?cluster=true")
  • This would work the same for the URL assigned to REDIS_OM_URL environment variable.

This resolves #408 and
resolves #412

@a9raag a9raag changed the title Support for Redis Cluster connection fix: Migrator throws Moved error when connecting to cluster Aug 4, 2023
@a9raag
Copy link
Author

a9raag commented Aug 7, 2023

Hi @chayim @simonprickett,
can you guys please take a look at this?

@simonprickett
Copy link
Contributor

Adding @dvora-h

@a9raag
Copy link
Author

a9raag commented Aug 14, 2023

We are planning to use redis-om in production and waiting for this fix.
Any input will be appreciated @dvora-h
Thanks :)

@XChikuX
Copy link

XChikuX commented Sep 11, 2023

@chayim @dvora-h I'd love this checked in. If you have the time.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to correct the spacing issues here.

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

Successfully merging this pull request may close these issues.

Unable to use redisearch on cluster Redis cluster Migrator not working
3 participants