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

Upgrade ES client python library to 8.x #200

Closed
ywangd opened this issue Feb 1, 2023 · 5 comments · Fixed by #210
Closed

Upgrade ES client python library to 8.x #200

ywangd opened this issue Feb 1, 2023 · 5 comments · Fixed by #210
Labels
enhancement New feature or request

Comments

@ywangd
Copy link
Owner

ywangd commented Feb 1, 2023

We are still using 7.17. It is time to upgrade to 8.x for either the full client library or just the transport library.

@ywangd ywangd added the enhancement New feature or request label Feb 1, 2023
@pquentin
Copy link

Hello! As the Python clients maintainer, I would definitely recommend upgrading elasticsearch-py itself. The transport library is more of a building block to allow building other client libraries, and will be more difficult to use.

I've also recently worked on making upgrading elasticsearch-py easier, see https://discuss.elastic.co/t/dec-13th-2023-en-10-reasons-to-upgrade-to-elasticsearch-py-8-x/347292 (item 4 in particular). The trick is waiting for elasticsearch-py 8.12 which should be out by the end of the week.

@ywangd
Copy link
Owner Author

ywangd commented Jan 17, 2024

Thanks Quentin! Peek does not use the query DSL so that I felt it might only need the transport library. In today's version, it reaches out to the transport field of the ES client object to perform request directly, e.g.:

peek/peek/connection.py

Lines 110 to 111 in 81faa51

return _wrapper(functools.partial(self.es.transport.perform_request, method, self.wrap_path(path)))(
body=payload, **kwargs

Given the above, do you still recommend elasticsearch-py over the transport library? Thanks

Btw, the end of this week sounds a perfect timing since spacetime is next week :)

@pquentin
Copy link

Oh, interesting. In that case, the bulk of the work will be instantiating a NodeConfig which is the main way Transport is configured. In practice, to use the transport library, you will need to:

Also note that as part of the upgrade, regardless of the library you use (transport or full client) you will have to:

  • stop using use_ssl (a scheme is now required in hosts)
  • stop patching _verified_elasticsearch (the check is done with headers in 8.x)
  • drop support for Python 3.6 (it went EOL two years ago)

ywangd added a commit that referenced this issue Jan 24, 2024
Upgrade the dependency to version 8 from 7. The dependency is now on the lower level transport library (elastic_transport) instead of the high level library (elasticsearch-py) since Peek does not use high level DSL.

Resolves: #200
@ywangd
Copy link
Owner Author

ywangd commented Jan 24, 2024

Thanks for the pointers, Quentin! The upgrade is now done with elastic_transport 8.12.0. I started with elasticsearch-py but it felt harder to control the underlying transport object for the things that I need. This is mostly expected. So I am settled with relying on the transport library directly.

I kept use_ssl since it is useful at Peek level to deal with bare address like localhost:9200.

@pquentin
Copy link

Awesome! Right, for use_ssl I meant that you would have to stop passing it to the library, but it makes sense to keep in your own code.

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

Successfully merging a pull request may close this issue.

2 participants