dilnazanlid commented on code in PR #73118:
URL: https://github.com/apache/airflow/pull/73118#discussion_r4004041274
##########
task-sdk/src/airflow/sdk/importers/python_importer.py:
##########
@@ -78,41 +135,43 @@ def can_handle(self, definition: DagDefinition | str |
Path) -> bool:
def list_dag_definitions(
self,
bundle: BaseDagBundle,
- *,
- safe_mode: bool = True,
- ) -> Iterator[DagDefinition]:
- """List Python DAG definitions in a bundle matching supported
extensions."""
- yield from find_file_dag_definitions(bundle.path,
self.supported_extensions, safe_mode=safe_mode)
+ ) -> Iterator[FileDagDefinition | ZipFileDagDefinition]:
+ """
+ List Python DAG files in a bundle matching supported extensions.
+
+ This does not look for members inside zip archives. Definition
discovery
+ of zip archives is done inside :class:`..zip_importer.ZipImporter`
+ instead.
+ """
+ if bundle.path.is_dir():
+ yield from find_file_dag_definitions(bundle.path,
self.supported_extensions)
Review Comment:
I am not sure here, but the method is said to return `ZipFileDagDefinition`
too, but `find_file_dag_definitions` only returns FileDagDefinition now.
Should `ZipFileDagDefinition` be added into the `find_file_dag_definitions` as
return type? As it could return all DagDefinition class children before, now it
might return nothing for python files within zip (and overall for all zip files
as the `find_file_dag_definitions` is used in the `zip_importer.py` too).
Or maybe make `ZipFileDagDefinition` subclass of `FileDagDefinition` but I
am not sure about it, up to you
##########
task-sdk/src/airflow/sdk/importers/python_importer.py:
##########
@@ -155,68 +209,50 @@ def get_source_code(self, definition: DagDefinition) ->
DagSourceCode:
source_code="# Sourceless bytecode (.pyc) — source code not
available\n",
language="python",
)
- return DagSourceCode(
- source_code=definition.read_text(encoding="utf-8"),
- language="python",
- )
-
- def might_contain_dag(self, file_path: str | Path, safe_mode: bool = True)
-> bool:
- """Check whether a file might contain Airflow DAGs according to safe
mode heuristics."""
- if not safe_mode:
- return True
- return might_contain_dag(str(file_path), safe_mode, conf=conf)
+ return
DagSourceCode(source_code=definition.read_text(encoding="utf-8"),
language="python")
def _load_modules_from_file(
Review Comment:
nit: maybe `_load_modules_from_definition`?
--
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]
