jscheffl commented on code in PR #57321:
URL: https://github.com/apache/airflow/pull/57321#discussion_r2468032734
##########
airflow-core/src/airflow/cli/commands/dag_command.py:
##########
@@ -361,30 +362,66 @@ def dag_list_dags(args, session: Session = NEW_SESSION)
-> None:
file=sys.stderr,
)
+ # Deprecation warning for --local flag
+ if getattr(args, "local", False):
+ warnings.warn(
+ "The --local flag is deprecated and will be removed in Airflow
4.0. "
+ "The command now automatically loads from filesystem when
appropriate.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+
dagbag_import_errors = 0
dags_list = []
- if args.local:
+ use_filesystem = False
+
+ # Priority-based source detection:
+ # 1. --dagfile-path (specific file)
+ # 2. --bundle-name (specific bundle(s))
+ # 3. Local filesystem (default DAGs folder)
+ # 4. Database (fallback)
+
+ dagfile_path = getattr(args, "dagfile_path", None)
+
+ if dagfile_path or args.bundle_name or getattr(args, "local", False):
+ # Explicitly requested filesystem source
+ use_filesystem = True
+ else:
+ # Check if database has any DAGs, otherwise use filesystem
+ dag_count =
session.scalar(select(func.count()).select_from(SerializedDagModel))
Review Comment:
That check includes creation of a new (sqlite in most cases) DB file to get
an empty list? Can this be "skipped" preventing the creation of sqlite file as
well?
##########
airflow-core/src/airflow/cli/commands/dag_command.py:
##########
@@ -459,29 +496,63 @@ def dag_details(args, session: Session = NEW_SESSION):
@provide_session
def dag_list_import_errors(args, session: Session = NEW_SESSION) -> None:
"""Display dags with import errors on the command line."""
+ # Deprecation warning for --local flag
+ if getattr(args, "local", False):
+ warnings.warn(
+ "The --local flag is deprecated and will be removed in Airflow
4.0. "
+ "The command now automatically loads from filesystem when
appropriate.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+
data = []
+ use_filesystem = False
- if args.local:
- # Get import errors from local areas
+ # Priority-based source detection:
+ # 1. --dagfile-path (specific file)
+ # 2. --bundle-name (specific bundle(s))
+ # 3. Local filesystem (default DAGs folder)
+ # 4. Database (fallback)
- if args.bundle_name:
+ dagfile_path = getattr(args, "dagfile_path", None)
+
+ if dagfile_path or args.bundle_name or getattr(args, "local", False):
+ # Explicitly requested filesystem source
+ use_filesystem = True
+ else:
+ # Check if database has any import errors, otherwise check filesystem
+ error_count =
session.scalar(select(func.count()).select_from(ParseImportError))
+ if error_count == 0:
+ use_filesystem = True
+
+ if use_filesystem:
+ # Get import errors from filesystem
+ if dagfile_path:
+ # Priority 1: Check specific file
+ dagbag = DagBag(dag_folder=dagfile_path, include_examples=False)
+ for filename, errors in dagbag.import_errors.items():
+ data.append({"filepath": filename, "error": errors})
+ elif args.bundle_name:
Review Comment:
This logic smells very redundant to above, can it (somehow) be consolidated
to have it only once?
--
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]
