Skip to content

Conversation

@esokullu
Copy link

Currently tcp scheme is not accepted. It is accepted with predis and others. For compatibility with other libraries, I suggest you include it as well.

@clue
Copy link
Owner

clue commented Jun 24, 2018

Thank you for your contribution!

The redis:// URI scheme has been added not too long ago via #60 and the (previously supported) tcp:// scheme has been removed via https://github.com/clue/php-redis-react/pull/61/files#diff-8157a0d28a58887c0293ce14b4b0281fL112.

I'm not sure I follow what the motivation for bringing this back is, so maybe you can elaborate? It's my understanding that the redis[s]:// URI schemes are standardized via https://www.iana.org/assignments/uri-schemes/prov/redis and https://www.iana.org/assignments/uri-schemes/prov/rediss respectively and I would rather see these supported in all Redis client library, no?

@nrk
Copy link

nrk commented Jun 26, 2018

@esokullu uniformity (rather than compatibility) is exactly the reason why the redis:// scheme was introduced and standardized, which also makes this the main reason why libraries should support this scheme and its rules before anything else. Historically other libraries using tcp as a scheme always parsed and interpreted parameters in the URI in their own way because there were no defined and fixed rules, so simply allowing tcp:// doesn't actually mean anything in this context.

@clue
Copy link
Owner

clue commented Sep 2, 2018

I'm closing this for now as it hasn't received any input in a while and I believe this has been answered. Please come back with more details if this problem persists and we can reopen this 👍

Either way, thank you for looking into this @esokullu!

@clue clue closed this Sep 2, 2018
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.

3 participants