8000 symfony/messager: how to set-up Redis transport connection by the socket? · Issue #34682 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

symfony/messager: how to set-up Redis transport connection by the socket? #34682

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.

Sign up for GitHub

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

Jump to bottom
Closed
alebedev80 opened this issue Nov 28, 2019 · 10 comments
Closed

symfony/messager: how to set-up Redis transport connection by the socket? #34682

alebedev80 opened this issue Nov 28, 2019 · 10 comments
Labels
Feature Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. Messenger

Comments

@alebedev80
Copy link
Contributor
alebedev80 commented Nov 28, 2019 •
edited
Loading

I need to set-up connection to the Redis service by the socket. How to set-up DSN?

By the code allows only DSN with valid url Connection.php:

if (false === $parsedUrl = parse_url($dsn)) {
    throw new InvalidArgumentException(sprintf('The given Redis DSN "%s" is invalid.', $dsn));
}

So DSN like "redis:///var/run/redis/redis.sock" is invalid.
Is there any reason for that or is it an architectural flaw?
@chalasr
Copy link
Member
chalasr commented Dec 2, 2019

Sounds good to me. Up for a PR?

@chalasr chalasr added Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. labels Dec 2, 2019
@mertingen
Copy link
Contributor
mertingen commented Dec 6, 2019 •
edited
Loading

@chalasr I am drastically interested in figuring it out. I totally got the problem and feature. Likewise, what way should I follow?

@flip111
Copy link
Contributor
flip111 commented Dec 11, 2019

Seems that "DSN" (Data source name aka "connection string") is not a specified format with a RFC behind it. To come up with such a format is quite some effort. Or even if you take the a format that already exist there is still the problem of parsing it correctly. Maybe better to rename the fromDsn function to fromUrl and then have another function fromSocket that just checks if it's a valid file on your system.

@mertingen
Copy link
Contributor

Yes, I could check if it's a valid file on the system but how can it connect by using socket file on rest of the code?

@flip111
Copy link
Contributor
flip111 commented Dec 11, 2019

It depends on the underlying driver ...

for example this driver https://github.com/phpredis/phpredis#example-1

@chalasr
Copy link
Member
chalasr commented Dec 11, 2019

The Cache component handles such DSNs (Lock does probably as well, I didn't check), doing the same for Messenger seems consistent.
Implementation-wise, I suggest to take inspiration from the Cache's RedisTrait.
The code area that needs to be patched is the Connection class from the Redis transport, and the related framework configuration probably needs to be updated accordingly.

@flip111
Copy link
Contributor
flip111 commented Dec 11, 2019

Perhaps there is an opportunity here to avoid some duplication of code in these two files

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Cache/Traits/RedisTrait.php

@JJarrie
Copy link
Contributor
JJarrie commented Jan 9, 2020 •
edited
Loading

T 8000 he Lock component use an already connected Redis client.

Basically to resolve it we need, use this part of RedisTrait (The scheme part are util?) to avoid error the error of parse_url:

if (0 === strpos($dsn, 'redis:')) {
            $scheme = 'redis';
        } elseif (0 === strpos($dsn, 'rediss:')) {
            $scheme = 'rediss';
        } else {
            throw new InvalidArgumentException(sprintf('Invalid Redis DSN: %s does not start with "redis:" or "rediss".', $dsn));
        }

        $dsn = preg_replace_callback('#^'.$scheme.':(//)?(?:(?:[^:@]*+:)?([^@]*+)@)?#', function ($m) use (&$auth) {
            if (isset($m[2])) {
                $auth = $m[2];
            }

            return 'file:'.($m[1] ?? '');
        }, $dsn);

And if we are this case, use the 'path' value (of parse_rul) and avoid to pass the port to method Redis::connect.

@JJarrie JJarrie mentioned this issue Jan 9, 2020
Closed
@JJarrie
Copy link
Contributor
JJarrie commented Jan 9, 2020

So I have push a little too fast, this issue have to be fix on master (feature)?

@JJarrie JJarrie mentioned this issue Jan 10, 2020
Merged
fabpot added a commit that referenced this issue Jan 16, 2020
@fabpot
feature #35295 [Messenger] Messenger redis local sock dsn (JJarrie)
af4035d 
This PR was squashed before being merged into the 5.1-dev branch (closes #35295).

Discussion
----------

[Messenger] Messenger redis local sock dsn

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #34682
| License       | MIT
| Doc PR        | symfony/symfony-docs#12924

Enable connection by unix socket to Redis.

``` messenger.yaml
framework:
    messenger:
        transports:
            async: "redis:///var/run/redis/redis.sock"
```

Commits
-------

0421e01 [Messenger] Messenger redis local sock dsn
@fabpot fabpot closed this as completed Jan 16, 2020
@alebedev80
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them. Messenger
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enable messenger on unix socket for redis
7 participants
@fabpot @alebedev80 @xabbuh @flip111 @chalasr @JJarrie @mertingen
0