jedcunningham commented on code in PR #45779:
URL: https://github.com/apache/airflow/pull/45779#discussion_r1943310556
##########
airflow/cli/commands/remote_commands/dag_command.py:
##########
@@ -346,8 +353,14 @@ def get_dag_detail(dag: DAG) -> dict:
dag_detail = _get_dagbag_dag_details(dag)
return {col: dag_detail[col] for col in valid_cols}
+ def filter_dags_by_bundle():
+ """Filter DAGs based on the specified bundle name, if provided."""
+ if args.bundle_name:
+ return [dag for dag in dagbag.dags.values() if
dag.get_bundle_name() == args.bundle_name]
+ return dagbag.dags.values()
+
AirflowConsole().print_as(
- data=sorted(dagbag.dags.values(), key=operator.attrgetter("dag_id")),
+ data=sorted(filter_dags_by_bundle(),
key=operator.attrgetter("dag_id")),
Review Comment:
```suggestion
data=sorted(filter_dags_by_bundle(dagbag.dags.values(),
args.bundle_name), key=operator.attrgetter("dag_id")),
```
part 2.
##########
airflow/cli/commands/remote_commands/dag_command.py:
##########
@@ -346,8 +353,14 @@ def get_dag_detail(dag: DAG) -> dict:
dag_detail = _get_dagbag_dag_details(dag)
return {col: dag_detail[col] for col in valid_cols}
+ def filter_dags_by_bundle():
+ """Filter DAGs based on the specified bundle name, if provided."""
+ if args.bundle_name:
+ return [dag for dag in dagbag.dags.values() if
dag.get_bundle_name() == args.bundle_name]
+ return dagbag.dags.values()
Review Comment:
```suggestion
def filter_dags_by_bundle(dags: list[DAG], bundle_name: str | None) ->
list[DAG]:
"""Filter DAGs based on the specified bundle name, if provided."""
if not bundle_name:
return dags
return [dag for dag in dags if dag.get_bundle_name() == bundle_name]
```
Let's pass in the dags and bundle name instead of relying on scope. Cleaner
and easier to grok.
(note: this does result in a query per dag to go find the bundle name, but
thats a short term problem until this is refactored to use the api anyways, so
not going to worry about it)
##########
tests/cli/commands/remote_commands/test_dag_command.py:
##########
@@ -334,12 +334,14 @@ def test_cli_list_dags_invalid_cols(self):
assert "Ignoring the following invalid columns: ['invalid_col']" in out
@conf_vars({("core", "load_examples"): "false"})
- def test_cli_list_dags_prints_import_errors(self):
- dag_path = os.path.join(TEST_DAGS_FOLDER, "test_invalid_cron.py")
- args = self.parser.parse_args(["dags", "list", "--output", "yaml",
"--subdir", dag_path])
- with contextlib.redirect_stderr(StringIO()) as temp_stderr:
- dag_command.dag_list_dags(args)
- out = temp_stderr.getvalue()
+ def test_cli_list_dags_prints_import_errors(self,
configure_testing_dag_bundle):
+ path_to_parse = TEST_DAGS_FOLDER / "test_invalid_cron.py"
+
+ args = self.parser.parse_args(["dags", "list", "--output", "yaml",
"--bundle-name", "testing"])
Review Comment:
You configure it just like below, with the `configure_testing_dag_bundle`
fixture. But just configuring it doesn't mean the DAGs will end up being parsed
and in the db.
Most of the dags list tests will need to be modified to account for reading
from the db instead of parsing on the fly. Could probably use the [get_test_dag
fixture](https://github.com/apache/airflow/blob/3cc5ca8d0e51205287587afc25bc8242b8047f79/tests_common/pytest_plugin.py#L1368)
for this test case.
##########
airflow/cli/commands/remote_commands/dag_command.py:
##########
@@ -328,7 +332,10 @@ def dag_list_dags(args, session=NEW_SESSION) -> None:
f"List of valid columns: {list(dag_schema.fields.keys())}",
file=sys.stderr,
)
- dagbag = DagBag(process_subdir(args.subdir))
+
+ dagbag = DagBag(read_dags_from_db=True)
+ dagbag.collect_dags_from_db()
+
if dagbag.import_errors:
Review Comment:
This won't work any longer, as collecting from db doesn't grab import
errors. You'll need to refactor this. Hopefully a test is failing for this too?
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
