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

Migrate elasticsearch client to 8.* (or latest 7.*) #31256

Closed
1 task done
potiuk opened this issue May 12, 2023 · 8 comments · Fixed by #33135
Closed
1 task done

Migrate elasticsearch client to 8.* (or latest 7.*) #31256

potiuk opened this issue May 12, 2023 · 8 comments · Fixed by #33135
Assignees
Labels
area:providers good first issue kind:task A task that needs to be completed as part of a larger issue provider:elasticsearch

Comments

@potiuk
Copy link
Member

potiuk commented May 12, 2023

Body

The eleasticsearch version we use is old (from 2021). It is eleasticsearch 7.* and since we are using elasticsearch-dsl, we cannot used elasticsearch 8 currently (first version of that was also released in 2021).

We are currently limited to < 7.15.0 additionally to being limited to 7, because 7.15.0 breaks our tests as we found out when we released some of the dependencies as part of #30067

We should migrate to a newer version of elasticsearch. There are two options:

  1. Fix the tests and migrate to lates 7.* version - and bump to latest released 7* by fixing our tests (possible for short term).
  2. Migreate to 8 version (better long term)

Committer

  • I acknowledge that I am a maintainer/committer of the Apache Airflow project.
@potiuk potiuk added the kind:meta High-level information important to the community label May 12, 2023
@potiuk potiuk changed the title Migrate elasticsearch to 8 Migrate elasticsearch client to 8.* (or latest 7.*) May 12, 2023
@Owen-CH-Leung
Copy link
Contributor

Since elasticsearch-dsl doesn't support elasticsearch 8, if we are to migrate to the official elasticsearch 8, does it mean we need to deprecate elasticsearch-dsl also ?

@potiuk
Copy link
Member Author

potiuk commented May 26, 2023

If we do yes. But that also might be reason why we don't do it.

@Owen-CH-Leung
Copy link
Contributor

Ok. Please assign this issue to me. I'll take some time to study the current usage of this package and propose whether to upgrade to 8.* or latest 7.*

@potiuk
Copy link
Member Author

potiuk commented May 27, 2023

Done.

@Owen-CH-Leung
Copy link
Contributor

I saw elasticsearch_dsl was only used in 3 files as follows (one usage in taskhandler & 2 in testing) :

Search(index=self.index_patterns, using=self.client)

"elasticsearch": ["elasticsearch", "es.elastic", "elasticsearch_dsl"],

with mock.patch("elasticsearch_dsl.Search.execute") as mock_execute:

Given the utilization of elasticsearch_dsl is fairly limited, I'll try to upgrade to elasticsearch 8.* and deprecate elasticsearch_dsl

@Owen-CH-Leung
Copy link
Contributor

@Owen-CH-Leung
Copy link
Contributor

I feel like if we are to upgrade to elasticsearch 8, we need to divide it into chunk of works (coz we will be deprecating 2 packages). Since elasticsearch-dbapi is used to construct the ElasticsearchSQLHook, we might first need to refactor it using the existing official elasticsearch library. Then we can go further to refactor the taskhandler that uses elasticsearch_dsl to use the existing elasticsearch library. Finally we can attempt to upgrade to 8, remove those 2 libraries, and fix the failing tests. What do you think ? @potiuk

@potiuk
Copy link
Member Author

potiuk commented Jun 8, 2023

sure

@eladkal eladkal added area:providers good first issue kind:task A task that needs to be completed as part of a larger issue provider:elasticsearch and removed kind:meta High-level information important to the community labels Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers good first issue kind:task A task that needs to be completed as part of a larger issue provider:elasticsearch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants