Use endpoints official client #1277
Conversation
There was a problem hiding this comment.
The more I see from this change the more I start to like it. Now in case things change on the API side either it is fixed "automatically" on the elasticsearch-php side or we get notified, that the method does not exist anymore.
Should we create a "meta" github issue to track the changes which are more complicated and are not done yet?
| @@ -406,7 +424,10 @@ public function addAlias($name, $replace = false) | |||
|
|
|||
| $data['actions'][] = ['add' => ['index' => $this->getName(), 'alias' => $name]]; | |||
|
|
|||
| return $this->getClient()->request($path, Request::POST, $data); | |||
| $endpoint = new Update(); | |||
There was a problem hiding this comment.
Do we also have to add the index name here?
There was a problem hiding this comment.
No, it is in "$data".
Actually here we need to use https://github.com/elastic/elasticsearch-php/blob/master/src/Elasticsearch/Endpoints/Indices/Alias/Put.php, but I don't quickly find a way to work with "$replace" argument there. Any suggestion?
There was a problem hiding this comment.
Not sure I can follow the above comment?
There was a problem hiding this comment.
I just mean that we need use Put endpoint there, but I don't find a way how to process $replace flag there. So, it can be tricky and need more time and can be done later. And asking maybe you see quick solution for this. So, we can do this later.
There was a problem hiding this comment.
Got it, also not sure how to deal with it. Lets merge it as is.
| @@ -420,11 +441,10 @@ public function addAlias($name, $replace = false) | |||
| */ | |||
| public function removeAlias($name) | |||
| { | |||
| $path = '_aliases'; | |||
| $endpoint = new \Elasticsearch\Endpoints\Indices\Alias\Delete(); | |||
| $endpoint->setName($name); | |||
There was a problem hiding this comment.
Do we also need the index name here? It was part of the request before ...
There was a problem hiding this comment.
No, it is not needed, since we call $this->requestEndpoint(...) it will add index name there.
lib/Elastica/Node/Info.php
Outdated
| foreach ($params as $param) { | ||
| $path .= $param.','; | ||
| } | ||
| $endpoint->setMetric(join(',', $params)); |
There was a problem hiding this comment.
Wow, I just checked official client code, "join" even not needed, we can call setMetric with array, https://github.com/elastic/elasticsearch-php/blob/master/src/Elasticsearch/Endpoints/Cluster/Nodes/Info.php#L31 . Will change it at evening and looks like we haven't test for this case.
|
I think "meta" is not needed. It is easy to find places that can be ported to Endpoints. Just check "usage" of "request" function in Type, Index and Client. For now mostly such places left in Client. |
|
Looks loke all done, ping @ruflin |
|
@ewgRa Merged. Thanks. |
* Official client usage for endpoints * drop unused use and use index requestEndpoint * better usage of setMetric
This PR start using Endpoints from official client.
There is still places where raw request used, but they require more work and can be done later, during some refactoring. For example official client for Bulk and similar multi requests require SerializerInterface, and there is research needed, can Elastica use Serializer from official client.
This PR cover most cases.