Taragolis commented on code in PR #38466:
URL: https://github.com/apache/airflow/pull/38466#discussion_r1537947062
##########
airflow/providers/docker/operators/docker.py:
##########
@@ -343,13 +345,21 @@ def hook(self) -> DockerHook:
assert_hostname=self.tls_hostname,
ssl_version=self.tls_ssl_version,
)
- return DockerHook(
- docker_conn_id=self.docker_conn_id,
- base_url=self.docker_url,
- version=self.api_version,
- tls=tls_config,
- timeout=self.timeout,
- )
+ hook = None
+ for url in self.docker_url:
+ hook = DockerHook(
+ docker_conn_id=self.docker_conn_id,
+ base_url=url,
+ version=self.api_version,
+ tls=tls_config,
+ timeout=self.timeout
+ )
+ try:
+ hook.get_conn()
+ return hook
+ except Exception as e:
+ self.log.error(f"Failed to establish connection to Docker host
{url}: {e}")
+ return hook
Review Comment:
In all URI are invalid than instead of raise an error it will return None,
which is incorrect behaiviour
##########
airflow/providers/docker/operators/docker.py:
##########
@@ -343,13 +345,21 @@ def hook(self) -> DockerHook:
assert_hostname=self.tls_hostname,
ssl_version=self.tls_ssl_version,
)
- return DockerHook(
- docker_conn_id=self.docker_conn_id,
- base_url=self.docker_url,
- version=self.api_version,
- tls=tls_config,
- timeout=self.timeout,
- )
+ hook = None
+ for url in self.docker_url:
+ hook = DockerHook(
+ docker_conn_id=self.docker_conn_id,
+ base_url=url,
+ version=self.api_version,
+ tls=tls_config,
+ timeout=self.timeout
+ )
+ try:
+ hook.get_conn()
Review Comment:
This might not raise an error here, because `DockerHook.api_client` didn't
do any specific tests and it is depend on whether or not `docker.APIClient`
call something from the docker daemon, e.g. this one would not switch to the
second uri
```python
from airflow.decorators import task
from airflow.models.dag import DAG
with DAG("pr_38466", schedule=None, tags=["38466", "docker", "docker_url"]):
@task.docker(
image="python:3.9-slim",
docker_url=["unix://foo/bar/spam.egg",
"unix://var/run/docker.sock",],
auto_remove="force",
api_version="1.30",
)
def f():
return (
"Apache Airflow™ is an open-source platform for developing, "
"scheduling, and monitoring batch-oriented workflows."
)
f()
```
--
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]
