jedcunningham commented on code in PR #46290:
URL: https://github.com/apache/airflow/pull/46290#discussion_r1935987483
##########
airflow/utils/file.py:
##########
@@ -254,7 +254,7 @@ def list_py_file_paths(
contains Airflow DAG definitions. If not provided, use the
core.DAG_DISCOVERY_SAFE_MODE configuration setting. If not set, default
to safe.
- :return: a list of paths to Python files in the specified directory
+ :return: a list of relative paths to Python files in the specified
directory
Review Comment:
Isn't this still returning absolute paths though?
##########
airflow/dag_processing/manager.py:
##########
@@ -102,10 +102,16 @@ class DagFileStat:
class DagFileInfo:
"""Information about a DAG file."""
- path: str # absolute path of the file
+ rel_path: Path
bundle_name: str
bundle_path: Path | None = field(compare=False, default=None)
Review Comment:
Is this able to be None for testing reasons? I feel like it should at least
not have a default of None. (I feel like I asked this elsewhere, but can't find
it...)
##########
airflow/dag_processing/collection.py:
##########
@@ -210,7 +210,7 @@ def _serialize_dag_capturing_errors(
except Exception:
log.exception("Failed to write serialized DAG dag_id=%s fileloc=%s",
dag.dag_id, dag.fileloc)
dagbag_import_error_traceback_depth = conf.getint("core",
"dagbag_import_error_traceback_depth")
- return [(dag.fileloc,
traceback.format_exc(limit=-dagbag_import_error_traceback_depth))]
+ return [(dag,
traceback.format_exc(limit=-dagbag_import_error_traceback_depth))]
Review Comment:
Good find, must not have good test coverage over this?
--
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]
