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

Improve Redis configuration #113

Closed
Alphix opened this issue Aug 20, 2021 · 4 comments · Fixed by #116
Closed

Improve Redis configuration #113

Alphix opened this issue Aug 20, 2021 · 4 comments · Fixed by #116

Comments

@Alphix
Copy link

Alphix commented Aug 20, 2021

The redis configuration allows a host and port to be set via the fast_cache_redis_host and fast_cache_redis_port variables.

However, it isn't currently possible to define a password and the database to use (see the Connecting to a database section here).

I tried hardcoding a password by modifying function __construct in app/libraries/MailSo/Cache/Drivers/Redis.php from:

        function __construct(string $sHost = '127.0.0.1', int $iPost = 6379, int $iExpire = 43200, string $sKeyPrefix = '')
        {
                $this->sHost = $sHost;
                $this->iPost = $iPost;
                $this->iExpire = 0 < $iExpire ? $iExpire : 43200;

                $this->oRedis = null;

                try
                {
                        $this->oRedis = new \Predis\Client('unix:' === substr($sHost, 0, 5) ? $sHost : array(
                                'host' => $sHost,
                                'port' => $iPost
                        ));

to:

        function __construct(string $sHost = '127.0.0.1', int $iPost = 6379, int $iExpire = 43200, string $sKeyPrefix = '')
        {
                $this->sHost = $sHost;
                $this->iPost = $iPost;
                $this->iExpire = 0 < $iExpire ? $iExpire : 43200;

                $this->oRedis = null;

                try
                {
                        $this->oRedis = new \Predis\Client('unix:' === substr($sHost, 0, 5) ? $sHost : array(
                                'host' => $sHost,
                                'port' => $iPost,
                                'password' => 'foobar`,
                                'database' => '0'
                        ));

...and it worked. Would it be possible to add two more variables (fast_cache_redis_password and fast_cache_redis_database) to snappymail?

@the-djmaze
Copy link
Owner

Can you push a PR or should i just change it for you?

@Alphix
Copy link
Author

Alphix commented Aug 21, 2021

My PHP skills are not the best. I'm trying to figure out what the best approach would be. Predis supports a URL style syntax which should be able to support all kinds of schemes (and it looks like the current code already looks for a host string which starts with unix:, as shown in the snippets above, so URL style syntax is halfway supported).

Problem is, so far I haven't been able to get the URL syntax to accept a password for unix: style connections...so I need to figure this out before I can do a proper PR.

Alphix added a commit to Alphix/snappymail that referenced this issue Aug 21, 2021
By using a connection URL, it is possible to support all Predis features, like
passwords, unix sockets, database selection, etc.

Tested with TCP and unix socket connections.

Closes the-djmaze#113
@the-djmaze
Copy link
Owner

Although i merged and closed this issue, i'm reverting your changes for compatibility.
The real bug was only in MailSo\Cache\Drivers\Redis class.

Trick will now become:
fast_cache_redis_host = 'tcp://127.0.0.1:6379'
fast_cache_redis_port = 0

I also made this change to Memcache driver.

@ossplus
Copy link

ossplus commented Apr 1, 2024

#1534

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 a pull request may close this issue.

3 participants