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

allow unix-socket connection in RedisSettings (socket_address) #271

Closed
wants to merge 10 commits into from
Closed

Conversation

lazToum
Copy link
Contributor

@lazToum lazToum commented Oct 18, 2021

following the recommendation in #209 (comment)

@samuelcolvin
Copy link
Member

I'm wondering how this will interact with #259? I want to get that merged before the next release.

@lazToum
Copy link
Contributor Author

lazToum commented Jan 26, 2022

It will probably need a few minor modifications. (adding a unix_socket_path init argument according to https://github.com/aio-libs/aioredis-py/blob/56d6b325ee246a3eb0fc8bb6803247c86bb2f494/aioredis/client.py#L845
Of course, I can work on it again later, after #259 is merged.

@samuelcolvin
Copy link
Member

#259 is merged, if you still want to work on this, I'd be happy to review an update.

@lazToum
Copy link
Contributor Author

lazToum commented Mar 13, 2022

Nice!
currently waiting for redis/redis-py#2046 (for UnixDomainSocketConnection) and yahoo/redislite#153 (for redislite/testing on python 3.10)

@lazToum
Copy link
Contributor Author

lazToum commented Mar 15, 2022

I think it's ready, as long as we have redis>=4.2.0rc3.
The actual changes (apart from the tests) are these:
https://github.com/lazToum/arq/blob/master/arq/connections.py#L42-L71
and
https://github.com/lazToum/arq/blob/master/arq/connections.py#L226-L233
Should I close this PR and start a new one on the updated master branch?

@samuelcolvin
Copy link
Member

There's lots of conflicts here, you'll need to rebase/merge with master.

@lazToum lazToum closed this Mar 28, 2022
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.

2 participants