Copilot commented on code in PR #59893:
URL: https://github.com/apache/airflow/pull/59893#discussion_r2651687485
##########
airflow-core/src/airflow/ui/src/pages/Dag/Dag.tsx:
##########
@@ -105,7 +105,13 @@ export const Dag = () => {
);
const { tabs: processedTabs } = useRequiredActionTabs({ dagId }, tabs, {
- refetchInterval,
+ refetchInterval:
+
+ (dag?.active_runs_count ?? 0) > 0 ||
+ (latestRun && isStatePending(latestRun.state))
+
+ ? refetchInterval
+ : false,
Review Comment:
This change appears unrelated to the PR's stated purpose of limiting public
API list responses for connections and variables. The modification changes the
refetchInterval logic for UI components, which does not align with the backend
API limiting functionality described in the PR description.
```suggestion
refetchInterval,
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/connections.py:
##########
@@ -125,7 +125,13 @@ def get_connections(
session: SessionDep,
connection_id_pattern: QueryConnectionIdPatternSearch,
) -> ConnectionCollectionResponse:
- """Get all connection entries."""
+
+ """Get connection entries."""
+ MAX_PUBLIC_API_LIMIT = 100
Review Comment:
The `MAX_PUBLIC_API_LIMIT` constant is defined locally within the function.
For consistency and maintainability, this constant should be defined at the
module level or in a shared configuration file, especially since the same
constant is used in the variables.py file. This would make it easier to adjust
the limit globally in the future.
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py:
##########
@@ -113,12 +113,17 @@ def get_variables(
variable_key_pattern: QueryVariableKeyPatternSearch,
) -> VariableCollectionResponse:
"""Get all Variables entries."""
+ MAX_PUBLIC_API_LIMIT = 100
+ if limit.value is not None and limit.value > MAX_PUBLIC_API_LIMIT:
+ limit.value = MAX_PUBLIC_API_LIMIT
+
+
variable_select, total_entries = paginated_select(
statement=select(Variable),
filters=[variable_key_pattern, readable_variables_filter],
order_by=order_by,
- offset=offset,
- limit=limit,
+ offset=offset.offset,
+ limit=limit.limit,
Review Comment:
The code is attempting to access `.offset` and `.limit` attributes on
`OffsetFilter` and `LimitFilter` objects, but these classes do not have such
attributes. These objects should be passed directly to `paginated_select` as
they are `OrmClause` instances with a `value` attribute. The correct usage is
`offset=offset` and `limit=limit` (as seen in the connections.py file and other
endpoints like pools.py).
```suggestion
offset=offset,
limit=limit,
```
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/variables.py:
##########
@@ -113,12 +113,17 @@ def get_variables(
variable_key_pattern: QueryVariableKeyPatternSearch,
) -> VariableCollectionResponse:
"""Get all Variables entries."""
+ MAX_PUBLIC_API_LIMIT = 100
Review Comment:
The `MAX_PUBLIC_API_LIMIT` constant is defined locally within the function.
For consistency and maintainability, this constant should be defined at the
module level or in a shared configuration file, especially since the same
constant is used in the connections.py file. This would make it easier to
adjust the limit globally in the future.
--
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]
