Add compatibility with elasticsearch >= 8.4 #5503
Conversation
| $client = $this->prophesize(Client::class); | ||
| $clientClass = class_exists(\Elasticsearch\Client::class) ? \Elasticsearch\Client::class : \Elastic\Elasticsearch\ClientInterface::class; | ||
|
|
||
| $client = $this->prophesize($clientClass); |
There was a problem hiding this comment.
Elastic\Elasticsearch\ClientInterface doesn't have cat method defined inside of it and it is not possible to mock method which does not exist. On other hand I couldn't rely on Elastic\Elasticsearch\Client as it is marked as final. ¯_(ツ)_/¯
So here I'm a bit stuck and don't see any good solution which will cover both >= 8.0 and <8.0 elasticsearch official clients.
p.s. For this reason all unit tests failed
There was a problem hiding this comment.
We should avoid changing existing tests, duplicate the tests you need, don't test the cat with es > 8
|
|
||
| $indexClient->delete(['index' => $index]); | ||
| if ($indexes !== []) { | ||
| $this->client->indices()->delete([ |
There was a problem hiding this comment.
remove all indexes within a single http request instead of multiple HTTP requests
|
this looks really awesome, I'll review on friday / this week-end can't for now but I've got your messages! |
sidz
changed the title
| $clientClass = class_exists(\Elasticsearch\Client::class) ? \Elasticsearch\Client::class : \Elastic\Elasticsearch\Client::class; | ||
|
|
||
| $clientDefinition = new Definition($clientClass); | ||
| $clientDefinition->setPublic(false); |
There was a problem hiding this comment.
| $client = $this->prophesize(Client::class); | ||
| $clientClass = class_exists(\Elasticsearch\Client::class) ? \Elasticsearch\Client::class : \Elastic\Elasticsearch\ClientInterface::class; | ||
|
|
||
| $client = $this->prophesize($clientClass); |
There was a problem hiding this comment.
We should avoid changing existing tests, duplicate the tests you need, don't test the cat with es > 8
src/Elasticsearch/Metadata/Document/Factory/CatDocumentMetadataFactory.php
Show resolved
Hide resolved
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Would love to see this ready for |
|
wow stale bot this wasn't supposed to be close :D sorry! |
|
|
yes someone needs to rebase and finish this I think that we should just remove the TypeHint for now within the providers so keep them as flexible as possible for the feature (just add a phpdoc). |
|
I'll do it |
|
ready for re-review @soyuka |
|
I'll continue this at #5795 |
4 participants
This PR adds elasticsearch 8 compatibility.
I've choose 8.4 as ClientInterface was introduced since 8.4 version and Client is marked as final.