Owen-CH-Leung commented on code in PR #71692:
URL: https://github.com/apache/airflow/pull/71692#discussion_r3826823096
##########
providers/opensearch/src/airflow/providers/opensearch/log/os_task_handler.py:
##########
@@ -219,9 +219,13 @@ def _create_opensearch_client(
) -> OpenSearch:
parsed_url = urlparse(_format_url(host))
resolved_port = port if port is not None else (parsed_url.port or 9200)
+ connection_kwargs: dict[str, Any] = {
+ "hosts": [{"host": parsed_url.hostname, "port": resolved_port,
"scheme": parsed_url.scheme}]
+ }
+ if username and password:
Review Comment:
This will change the client behaviour. If only one of `username` and
`password` is set, the opensearch client will silently connects unauthenticated
instead of failing with a clear 401 error.
##########
providers/opensearch/tests/unit/opensearch/log/test_os_task_handler.py:
##########
@@ -288,6 +288,22 @@ def test_client_with_patterns(self):
)
assert handler.index_patterns == patterns
+ def test_client_no_auth(self):
+ handler = OpensearchTaskHandler(
+ base_log_folder=self.local_log_location,
+ end_of_log_mark=self.end_of_log_mark,
+ write_stdout=self.write_stdout,
+ host="localhost",
+ port=9200,
+ username="",
+ password="",
+ json_format=self.json_format,
+ json_fields=self.json_fields,
+ host_field=self.host_field,
+ offset_field=self.offset_field,
+ )
+ assert "http_auth" not in handler.client.transport.kwargs
Review Comment:
This unit test only covers the case that both are empty but not the "one
set, one empty" case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
