Allow initialize connections from client object

[aguvillalba]

Thank you for your library! I have found the following issue:

Right now, if you create a client object with empty parameters like:

$client = new Client();
$client->setConfigValue('host','192.168.1.3');
$client->setConfigValue('port',9200);

Then it is imposible to initialize the connections of the client because the method Client::_initConnections is protected.

You allow to set config values but you only initializes the connections of the client in the constructor of the class. You should allow to initialize the connections after setting the config values too. So, the method initConnections should be public.

With this setup, it is imposible to use dependency injection by injecting your client without parameters because it will be impossible to initialize the connections later.

Thank you in advance!
Agustin.

[ewgRa]

good point.

Actually *Config methods I think mostly used for setup some configuration parameters that used in updateDocument and others.

You can do what you want by getConnections - setConnections. It will be work, but it is looks not so good.

Best way I think to have connectionConfig (maybe move it to ConnectionPool, or something like this) and just config, or even not to save connection config at all. Or move methods related to configuration connections to ConnectionPool, or something like this.

Is "getConnections - setConnections" will work for you?

[aguvillalba]

Hi ewgRa,

Thank you for your reply!

I think using "setConnections" is not a good idea, because it would mean that I have to create the $connection object in my code and then inject it into the $client object, and this implies that my code would have to deal with the creation of connections which it should not be my duty. My code should know nothing about client's connections to the ES server and etc.

Actually, I think that instead of changing the visibility of the method initConnections, we could just add a new public method called "connect" which just calls this protected method. Is an easy and consistent solution, because if it is allowed to set each connection parameter separately, then I should be able to trigger the connection to the server.

I have already forked this project and I will make the change and then will make a pull request. I think it could be a good improvement.

Best regards,
Agustin.

[aguvillalba]

There is a new pull-request (#1077) for this issue.

[ruflin]

Closing as #1077 was merged.