Skip to content

Added example for connecting to unix socket #68

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

Closed
wants to merge 1 commit into from
Closed

Conversation

cebe
Copy link

@cebe cebe commented Dec 18, 2017

I was searching for a way to connect to redis via unix socket and I found it a bit too hard to find, so I added this here.

@clue
Copy link
Owner

clue commented Dec 18, 2017

Good catch, thank you for this contribution!

The code LGTM, but it makes me wonder if it makes more sense to just integrate this into this library. I was actually planning to integrate this somewhat similar to clue/reactphp-socks#69 and possibly use the redis+unix:// URI scheme here.

What do you think about this? 👍

@cebe
Copy link
Author

cebe commented Dec 18, 2017

of course it would make sense to add direct support for it but there are a few things that are unclear about how to handle it:

  1. as you said, the scheme. redis+unix:// would be an option, I assume there is no standard for this?
    Also is the redis part needed? would unix:// be sufficient?
  2. how to specify the database? redis+unix:///var/run/redis/redis.sock?db=2 would probably work, redis+unix:///var/run/redis/redis.sock/2 not so much as it is indistinguishable from a path.
  3. specifying auth on a socket is pointless so its fine to skip that option imo.

@cebe
Copy link
Author

cebe commented Dec 18, 2017

Btw, I really like how much documentation is added for all the react libraries, thanks for that! :)

@clue
Copy link
Owner

clue commented Jan 24, 2018

Thank you for sparking this discussion and for your positive feedback, this has just been resolved via #70! :shipit:

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

Successfully merging this pull request may close these issues.

2 participants