ephraimbuddy commented on code in PR #59430:
URL: https://github.com/apache/airflow/pull/59430#discussion_r2626462173
##########
airflow-core/src/airflow/dag_processing/collection.py:
##########
@@ -371,7 +374,10 @@ def update_dag_parsing_results_in_db(
warnings: set[DagWarning],
session: Session,
*,
- warning_types: tuple[DagWarningType] = (DagWarningType.NONEXISTENT_POOL,),
+ warning_types: tuple[DagWarningType, DagWarningType] = (
Review Comment:
```suggestion
warning_types: tuple[DagWarningType, ...] = (
```
we should use this everywhere for warning_types so we can avoid signature
change in the future
##########
airflow-core/src/airflow/dag_processing/processor.py:
##########
@@ -200,20 +201,24 @@ def _parse_file_entrypoint():
sys.path.append(bundle_root)
result = _parse_file(msg, log)
+
if result is not None:
comms_decoder.send(result)
def _parse_file(msg: DagFileParseRequest, log: FilteringBoundLogger) ->
DagFileParsingResult | None:
# TODO: Set known_pool names on DagBag!
+ warning_statement = check_dag_file_static(os.fspath(msg.file))
Review Comment:
We should have a catch all exceptions here to ensure this doesn't break the
parsing. We should manually test this implementation with very large dag file.
##########
airflow-core/src/airflow/models/dagwarning.py:
##########
@@ -103,3 +103,4 @@ class DagWarningType(str, Enum):
ASSET_CONFLICT = "asset conflict"
NONEXISTENT_POOL = "non-existent pool"
+ PARSING_ERROR = "parsing error"
Review Comment:
Maybe RUNTIME_VARYING_VALUE
##########
airflow-core/src/airflow/dag_processing/processor.py:
##########
@@ -509,10 +513,6 @@ def start( # type: ignore[override]
client: Client,
**kwargs,
) -> Self:
- logger = kwargs["logger"]
-
- _pre_import_airflow_modules(os.fspath(path), logger)
Review Comment:
Why this change??
--
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]
