Skip to content

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