ephraimbuddy commented on code in PR #42320:
URL: https://github.com/apache/airflow/pull/42320#discussion_r1768443794
##########
airflow/api_fastapi/views/public/dags.py:
##########
@@ -35,39 +42,29 @@
@dags_router.get("/dags")
async def get_dags(
*,
- limit: int = 100,
- offset: int = 0,
- tags: Annotated[list[str] | None, Query()] = None,
- dag_id_pattern: str | None = None,
- only_active: bool = True,
- paused: bool | None = None,
- order_by: str = "dag_id",
+ limit: QueryLimit,
+ offset: QueryOffset,
+ tags: QueryTagsFilter,
+ dag_id_pattern: QueryDagIdPatternSearch,
+ only_active: QueryOnlyActiveFilter,
+ paused: QueryPausedFilter,
+ order_by: Annotated[SortParam, Depends(SortParam(["dag_id"]))],
session: Annotated[Session, Depends(get_session)],
) -> DAGCollectionResponse:
"""Get all DAGs."""
- allowed_sorting_attrs = ["dag_id"]
dags_query = select(DagModel)
- if only_active:
- dags_query = dags_query.where(DagModel.is_active)
- if paused is not None:
- if paused:
- dags_query = dags_query.where(DagModel.is_paused)
- else:
- dags_query = dags_query.where(~DagModel.is_paused)
- if dag_id_pattern:
- dags_query =
dags_query.where(DagModel.dag_id.ilike(f"%{dag_id_pattern}%"))
+
+ dags_query = apply_filters_to_query(dags_query, [only_active, paused,
dag_id_pattern, tags])
# TODO: Re-enable when permissions are handled.
# readable_dags = get_auth_manager().get_permitted_dag_ids(user=g.user)
# dags_query = dags_query.where(DagModel.dag_id.in_(readable_dags))
- if tags:
- cond = [DagModel.tags.any(DagTag.name == tag) for tag in tags]
- dags_query = dags_query.where(or_(*cond))
-
total_entries = get_query_count(dags_query, session=session)
- dags_query = apply_sorting(dags_query, order_by, {}, allowed_sorting_attrs)
- dags = session.scalars(dags_query.offset(offset).limit(limit)).all()
+
+ dags_query = apply_filters_to_query(dags_query, [order_by, offset, limit])
+
+ dags = session.scalars(dags_query).all()
Review Comment:
In sqlalchemy 2, which we are now using, query is no longer the right word;
select is being used, and I think we should follow it since we are writing a
new API. No strong opinion here, but when you look at sections like
https://docs.sqlalchemy.org/en/20/orm/queryguide/select.html, you discover that
the new term is select. If we are to use the select, then names like
`dags_query`, `apply_filters_to_query` should be changed to `dags_select`,
`apply_filters_to_select` etc. And looking at it, it's not until line 67 that
we performed a query to the DB
--
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]
