uranusjr commented on a change in pull request #21230:
URL: https://github.com/apache/airflow/pull/21230#discussion_r795404605
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -159,26 +158,42 @@ def _read(self, ti, try_number, metadata=None):
except Exception as f:
log += f'*** Unable to fetch logs from worker pod
{ti.hostname} ***\n{str(f)}\n\n'
else:
+ timeout = None # No timeout
+ try:
+ timeout = conf.getint('webserver', 'log_fetch_timeout_sec')
+ except (AirflowConfigException, ValueError):
+ pass
Review comment:
```suggestion
try:
timeout: Optional[int] = conf.getint('webserver',
'log_fetch_timeout_sec')
except (AirflowConfigException, ValueError):
timeout = None # No timeout
```
##########
File path: airflow/utils/log/file_task_handler.py
##########
@@ -19,18 +19,17 @@
import logging
import os
from pathlib import Path
-from typing import TYPE_CHECKING, Optional
+from typing import Optional
import httpx
from itsdangerous import TimedJSONWebSignatureSerializer
from airflow.configuration import AirflowConfigException, conf
+from airflow.models import TaskInstance
from airflow.utils.context import Context
from airflow.utils.helpers import parse_template_string,
render_template_to_string
from airflow.utils.log.non_caching_file_handler import NonCachingFileHandler
-
-if TYPE_CHECKING:
- from airflow.models import TaskInstance
Review comment:
Better to keep this conditional to avoid potential circuliar import.
`airflow.models` imports _way_ too many things it’s not a good idea to import
it in `utils`.
--
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]
