Upgrade ES library #705
Upgrade ES library #705
Conversation
From: https://www.elastic.co/guide/en/elasticsearch/client/python-api/current/release-notes.html#rn-8-0-0 - Removed the default URL of http://localhost:9200 due to Elasticsearch 8.0 default configuration being https://localhost:9200. The client’s connection to Elasticsearch now must be specified with scheme, host, and port or with the cloud_id parameter
From: https://www.elastic.co/guide/en/elasticsearch/client/python-api/current/release-notes.html#rn-8-0-0 - Added the meta property to ApiError to access the HTTP response metadata of an error. - Changed exception hierarchy, the major change is a new exception ApiError which differentiates between an error that’s raised from the transport layer (previously elasticsearch.exceptions.TransportError, now elastic_transport.TransportError) and one raised from the API layer
| elasticsearch==6.3.1 | ||
| elastic-transport==8.11.0 | ||
| # via elasticsearch | ||
| elasticsearch==8.11.1 |
There was a problem hiding this comment.
Is there a reason for this to be in a version from 2018?
Any issues about our actual ES server version? I don't think this was pinned to one particular version so it doesn't seem intentional.
There was a problem hiding this comment.
It looks like Rob was not happy with the fact the Bouncer's tests apparently mock Elasticsearch, and saw little value in updating the library anyway.
Bouncer's a pretty simple app so it shouldn't be too difficult to test it manually.
Could perhaps rewrite the tests to not mock Elasticsearch as well although that may be difficult since Bouncer talks directly to h's Elasticsearch. For example it's code in h, not Bouncer, that adds the documents to Elasticsearch that Bouncer reads.
There was a problem hiding this comment.
Could perhaps rewrite the tests to not mock Elasticsearch as well although that may be difficult since Bouncer talks directly to h's Elasticsearch.
Another option is that h handles talking to ES and exposes API for bouncer to use (with suitable authentication). That's assuming that h can't just get the data needed from Postgres instead. It's less efficient, but it means all the knowledge about ES structure would be in one codebase.
|
This is broken, at least by this error:
See elastic/elasticsearch-py#846 We'll have to check if we can upgrade to 8.0.0 without updating the server version. |
Found this while doing the python upgrade, much better to do it independently of that.
See commits for a little bit details on the changes.
Not sure why we never got this PR from dependabot though.