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.

Sign up for GitHub

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

Cleanup code for elasticsearch<8 #35707

Merged
merged 3 commits into from
Dec 1, 2023
Merged

Cleanup code for elasticsearch<8 #35707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
085e1b7
update elasticsearch doc for `use_ssl` parameter
eladkal Nov 17, 2023
77e32e4
Remove outdated support for elasticsearch<8
eladkal Nov 25, 2023
a7f0150
bring back retry_timeout
eladkal Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions airflow/providers/elasticsearch/log/es_task_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,17 @@ def get_es_kwargs_from_config() -> dict[str, Any]:
if elastic_search_config
else {}
)
# For elasticsearch>8 retry_timeout have changed for elasticsearch to retry_on_timeout
# in Elasticsearch() compared to previous versions.
# Read more at: https://elasticsearch-py.readthedocs.io/en/v8.8.2/api.html#module-elasticsearch
# TODO: Remove in next major release (drop support for elasticsearch<8 parameters)
if (
elastic_search_config
and "retry_timeout" in elastic_search_config
and not kwargs_dict.get("retry_on_timeout")
):
warnings.warn(
"retry_timeout is not supported with elasticsearch>=8. Please use `retry_on_timeout`.",
AirflowProviderDeprecationWarning,
stacklevel=2,
)
retry_timeout = elastic_search_config.get("retry_timeout")
if retry_timeout is not None:
kwargs_dict["retry_on_timeout"] = retry_timeout
Expand Down
1 change: 0 additions & 1 deletion docs/apache-airflow-providers-elasticsearch/logging/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ cert, etc.) use the ``elasticsearch_configs`` setting in your ``airflow.cfg``
remote_logging = True

[elasticsearch_configs]
use_ssl=True
verify_certs=True
ca_certs=/path/to/CA_certs

Expand Down
5 changes: 0 additions & 5 deletions tests/providers/elasticsearch/log/test_es_task_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,14 +671,11 @@ def test_retrieve_config_keys():
"""
with conf_vars(
{
("elasticsearch_configs", "use_ssl"): "True",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to keep it in the tests for check that we do not use non-existed parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which parameter are you worried about?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

("elasticsearch_configs", "use_ssl"): "True", This mostly for check that we remove it from resulting arguments here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but we no longer need it because we are on elasticsearch>7
the only reference to this parameter was in the docs which was wrong

("elasticsearch_configs", "http_compress"): "False",
("elasticsearch_configs", "timeout"): "10",
}
):
args_from_config = get_es_kwargs_from_config().keys()
# use_ssl is removed from config
assert "use_ssl" not in args_from_config
# verify_certs comes from default config value
assert "verify_certs" in args_from_config
# timeout comes from config provided value
Expand All @@ -698,8 +695,6 @@ def test_retrieve_retry_on_timeout():
}
):
args_from_config = get_es_kwargs_from_config().keys()
# use_ssl is removed from config
assert "retry_timeout" not in args_from_config
# verify_certs comes from default config value
assert "retry_on_timeout" in args_from_config

Expand Down