-
Notifications
You must be signed in to change notification settings - Fork 119
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
add elasticsearch 8.x.x support #244
base: master
Are you sure you want to change the base?
Conversation
61c8961
to
cbeb977
Compare
I rebased this one on recent master and added stuff to authors and changelog as suggested in the contribution guide. Ran |
How that I think of this more, considering that doc types are completely optional since es7, and I don't want to support really old stuff, I think the better way to do this is to completely remove doc_type and just require es7 as the minimum client version. |
Makes sense. I can constrain the elastic version to > 7 and make changes to remove supplying |
Yes, thank you.
…On Mon, Dec 18, 2023, 11:23 Çağlar Kutlu ***@***.***> wrote:
How that I think of this more, considering that doc types are completely
optional since es7, and I don't want to support really old stuff, I think
the better way to do this is to completely remove doc_type and just require
es7 as the minimum client version.
Makes sense. I can constrain the elastic version to > 7 and make changes
to remove supplying doc_type argument altogether in this MR then. Are you
okay with it?
—
Reply to this email directly, view it on GitHub
<#244 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA7TXPKDXVT57U2NJZS5KLYKADPPAVCNFSM6AAAAAAZ5MV3XOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZHA3TMNBSGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sorry for the delay, just got around to doing this. Following the discussion in elastic/elasticsearch-py#1698, I went with dropping support for below 7.15.0. Removed most of the version checks I introduced and simplified the changes. There is still a version check for ES 7 in one place. This was needed because 8.x deprecates the |
|
This should be now hopefully good to go @ionelmc |
🤦 Argh, failed again. Okay, so there were two problems:
I am not sure how to go with the 2nd issue. 30 minutes should be more than enough given that py311-pytest73-nodist-cover (windows) runs in 8 minutes. Somehow things become 4x slower with Python 3.12. We could either increase the timeout a bit (40 min?), or maybe get rid of pytest 7.3 support since py312-pytest7.4 seem to complete within 25 min. What do you think @ionelmc ? |
Elasticsearch python api deprecates the
doc_type
argument in theindex
function in v8.x.x. This is a minor fix for that.