Add Username, Password, Database and Unix Socket support. #352
Conversation
admin/class-nginx-helper-admin.php
Outdated
| 'redis_unix_socket' => '', | ||
| 'redis_database' => 0, | ||
| 'redis_username' => '', | ||
| 'redis_password' => '', |
admin/class-nginx-helper-admin.php
Outdated
| $data['redis_hostname'] = RT_WP_NGINX_HELPER_REDIS_HOSTNAME; | ||
| $data['redis_port'] = RT_WP_NGINX_HELPER_REDIS_PORT; | ||
| $data['redis_prefix'] = RT_WP_NGINX_HELPER_REDIS_PREFIX; | ||
| $data['redis_database'] = defined('RT_WP_NGINX_HELPER_REDIS_DATABASE') ? RT_WP_NGINX_HELPER_REDIS_DATABASE : 0; |
|
|
||
| $hostname = empty( $nginx_helper_admin->options['redis_unix_socket'] ) ? $nginx_helper_admin->options['redis_hostname'] : $nginx_helper_admin->options['redis_unix_socket']; | ||
| $port = empty( $nginx_helper_admin->options['redis_unix_socket'] ) ? $nginx_helper_admin->options['redis_port'] : 0; | ||
|
|
||
| $this->redis_object->connect( |
There was a problem hiding this comment.
On my local setup, there’s no Nginx server or Redis installed. When I switch branches from master to your branch, the code throws a fatal error. The plugin is trying to connect, but the connection is being refused. If the plugin is attempting to make a connection, there should also be a fatal error in the master branch.
Please take a look into this
There was a problem hiding this comment.
@SH4LIN I am unable to replicate the issue on my end. Can you please share the error here if possible?
There was a problem hiding this comment.
We are throwing a fatal error when the Redis connection is refused, which is understandable. However, this makes the entire admin side inaccessible. As a result, there is no way to update the connection configuration except by manually updating the value in the database or using WP-CLI.
One solution is to wrap the $this->redis_object->connect inside try catch and on the catch display WP notice
There was a problem hiding this comment.
@SH4LIN I found where the issue was and have fixed it. In predis it does not have checks for connection and instead immeditely executes the statement without checking if a connection is already established or not.
So in class-predis.php where we have addded selecting the databse feature, it was selecting the database without checking for connection establishment causing it to throw error.
| @@ -22,6 +22,12 @@ | |||
| 'redis_hostname', | |||
| 'redis_port', | |||
| 'redis_prefix', | |||
| 'redis_database', | |||
| </td> | ||
| </tr> | ||
| <tr> |
There was a problem hiding this comment.
Fix the indentation and use tabs instead of the space
|
@Vedant-Gandhi Is this ready for review? |
|
@SH4LIN Yes |
Description
The aim of this PR is to add support for including the Username, Password, Database and Unix Socket Path for Redis.
Type
Reference Issue
Screenshots
Testing
The code has been tested locally and is working as expected.