cakephp / elastic-search

Elastic search datasource for CakePHP
Other
88 stars 53 forks source link

Elastica's Client::_log deprecated in favor of Client::setLogger(LoggerInterface) #157

Closed josbeir closed 6 years ago

josbeir commented 6 years ago

Currently the Connection class extends the Elastica Client class and overrides the _log function. As stated in the _log doc block overriding this method is deprecated in favour of using a LoggerInterface set with setLogger().

This is where problems start with the current implementation. Cake uses setLogger/getLogger these days on connection classes, Loggers set with these methods are not always of the LoggerInterface kind (certainly not the QueryLogger/DebugLog from debugkit, these are QueryLogger's).

So logging queries using debug_kit for instance (which uses QueryLogger's, a non psr LoggerInterface compliant logger on the connection) immediately throws an exception:

Argument 1 passed to Elastica\Client::setLogger() must implement interface Psr\Log\LoggerInterface, instance of DebugKit\Database\Log\DebugLog

Overriding the setLogger on the plugins Connection method and removing the LoggerInterface type of course gives us type errors in strict mode.

So i'm not seeing a 2 line code solution on how to fix this as we are not allowed to override the setLogger method. So suggestions are welcome on how to create a proper fix for this.

Here's an idea idea from the top of my head:

josbeir commented 6 years ago

Implemented this idea on a separate branch here, i'll merge with #156 if you guys are ok with it. Its working pretty good :-)

markstory commented 6 years ago

Create an instance of Elastica client inside of connection and use php's magic __call to pass methods the client instance after checking they exist, we catch de logger and pass it to a custom psr logger, then set it using $this->_client->seTLogger()

Seems like a reasonable solution that helps avoid other method shadowing issues that come up in the future.

josbeir commented 6 years ago

Merged into #156

ddgreen commented 6 years ago

I experienced same probrem when I upgraded 3.5.16 => 3.6.4 CakePHP:3.6.4 CakePHP/elastic-search:@stable

lorenzo commented 6 years ago

Try upgrading to 1.5+

ddgreen commented 6 years ago

Thank you. But, the problem is not solved. CakePHP:3.6.4 CakePHP/elastic-search:1.5.2 (I'm using composer)

lorenzo commented 6 years ago

Do you think you can submit an issue with the fix? I know this was fixed in the master branch, but the master branch is working with elastic 6.

Maybe try to see what can be changed in 1.5 to make it work correctly if you can help us

ddgreen commented 6 years ago

Sorry, I resolved it when I upgraded 1.5.2 => 2.0.0-RC1