Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elasticsearch ^8.0 support #239

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

jayjfletcher
Copy link

This pull request contains breaking changes.

Elasticsearch 8.x introduced many breaking changes to the client. It seems to have been a complete overhaul.

8.x also introduced the ClientInterface so there is no longer a need for the following files:

  • src/Infrastructure/Elastic/ElasticClientFactory.php - removed
  • src/Infrastructure/Elastic/ElasticClientBuilder.php- removed

and associated tests:

  • tests/Unit/ElasticClientBuilderTest.php - removed
  • tests/Unit/ElasticClientFactoryTest.php - removed

Client can now be injected via ClientInterface so ElasticClientBuilder was replaced with Elastic's native ClientBuilder and documentation has been updated to reflect this.

@Jeroen-G
Copy link
Owner

Jeroen-G commented Apr 7, 2024

Whoah, you have put in a good amount of work! Thanks, I have approved the CI actions to run and give you some feedback and I will review the code asap.

@Jeroen-G
Copy link
Owner

Jeroen-G commented Apr 8, 2024

Re: the CI failures, check out the Makefile which can possibly fix the codestyle (and run the tests etc.) for you.

@sporchia
Copy link

@Jeroen-G I have been working on updating this PR to get 8.x into the lib. A few things related to CI, looks like there will need to be an update to a fair bit more as the Client class is the correct class to use, since ClientInterface doesn't have all the correct methods and static analysis. the Client class is now marked as final so as ES suggests the tests will have to be updated to mock the underlying HTTP requests instead of their Client class.

Long story short, would you rather I create a PR into this PR, or just have a fresh PR (with these initial changes in it and the new adjustments)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants