Copilot commented on code in PR #61448:
URL: https://github.com/apache/airflow/pull/61448#discussion_r2892798549
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -494,6 +495,11 @@ def trigger_dag_run(
current_user_id = user.get_id()
dag_run.note = (dag_run_note, current_user_id)
return dag_run
+ except BundleVersionUnavailable as e:
+ raise HTTPException(
+ status.HTTP_503_SERVICE_UNAVAILABLE,
+ f"Bundle version not yet available. Please retry: {e}",
Review Comment:
This endpoint returns a plain-string `detail` for
`BundleVersionUnavailable`, while the execution API returns a structured
`{reason, message}` object for the analogous 503. For client consistency,
consider returning a structured `detail` here too (e.g. include a stable
`reason` key) so UI/SDK consumers can reliably detect and handle this retryable
condition.
```suggestion
{
"reason": "BUNDLE_VERSION_UNAVAILABLE",
"message": f"Bundle version not yet available. Please retry:
{e}",
},
```
##########
airflow-core/src/airflow/serialization/definitions/dag.py:
##########
@@ -1097,6 +1099,116 @@ def get_edge_info(self, upstream_task_id: str,
downstream_task_id: str) -> EdgeI
return empty
+def _resolve_bundle_version(
+ dag: SerializedDAG,
+ session: Session,
+) -> str | None:
+ """
+ Resolve the bundle version for a DAG run based on the precedence hierarchy.
+
+ Precedence (highest to lowest):
+ 1. DAG-level configuration (dag.run_on_latest_version)
+ 2. Global configuration (core.run_on_latest_version)
+ 3. System default (use original version from DagModel)
+
+ :param dag: The serialized DAG
+ :param session: Database session
+ :return: The resolved bundle version, or None if versioning is disabled
+ """
+ if dag.disable_bundle_versioning:
+ return None
+
+ # Determine whether to use latest version based on precedence hierarchy
+ use_latest = _should_use_latest_version(dag)
+ source = _get_config_source(dag)
+
+ if use_latest:
+ log.debug("Using latest bundle version", dag_id=dag.dag_id,
source=source)
+ return _get_latest_bundle_version(dag.dag_id, session)
+
+ log.debug("Using original bundle version", dag_id=dag.dag_id,
source=source)
+ return _get_original_bundle_version(dag.dag_id, session)
+
+
+def _should_use_latest_version(
+ dag: SerializedDAG,
+) -> bool:
+ """
+ Determine whether to use latest bundle version based on precedence
hierarchy.
+
+ Returns True if latest version should be used, False otherwise.
+ """
+ # Level 1: DAG-level configuration (explicit True or False)
+ if dag.run_on_latest_version is not None:
+ return dag.run_on_latest_version
+
+ # Level 2: Global configuration (fallback to False)
+ return airflow_conf.getboolean("core", "run_on_latest_version",
fallback=False)
+
+
+def _get_config_source(
+ dag: SerializedDAG,
+) -> str:
+ """Return descriptive source of the bundle version configuration for
logging."""
+ if dag.run_on_latest_version is not None:
+ return "DAG configuration"
+ if airflow_conf.has_option("core", "run_on_latest_version"):
+ return "global configuration"
+ return "system default"
+
+
+def _get_latest_bundle_version(dag_id: str, session: Session) -> str | None:
+ """
+ Get the latest bundle version from DagBundleModel, falling back to
DagModel.
+
+ :param dag_id: The DAG ID
+ :param session: Database session
+ """
+ from airflow.models.dagbundle import DagBundleModel
+
+ dag_model = session.scalar(select(DagModel).where(DagModel.dag_id ==
dag_id))
+ if not dag_model:
+ log.warning(
+ "Cannot resolve latest bundle version: DagModel not found",
+ dag_id=dag_id,
+ )
Review Comment:
These `log.debug(..., dag_id=..., source=...)` / `log.warning(...,
dag_id=...)` calls pass arbitrary keyword arguments, which will raise
`TypeError` with standard `logging.Logger`. Use `%s` formatting args or
`extra={...}` instead (consistent with other logging in this file that uses
printf-style args).
##########
airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearTaskInstanceDialog.tsx:
##########
@@ -245,7 +248,7 @@ const ClearTaskInstanceDialog = ({ onClose: onCloseDialog,
open: openDialog, tas
open={open}
preventRunningTask={preventRunningTask}
/>
- ) : null}
+ ) : undefined}
Review Comment:
In JSX, returning `null` is the conventional way to render nothing;
returning `undefined` can trip linting rules and is less idiomatic. Prefer `) :
null}` here for consistency with other components.
```suggestion
) : null}
```
##########
airflow-core/src/airflow/ui/src/components/Clear/useRunOnLatestVersion.ts:
##########
@@ -0,0 +1,94 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useState } from "react";
+
+import { useConfig } from "src/queries/useConfig";
+
+type UseRunOnLatestVersionProps = {
+ /**
+ * Current DAG bundle version
+ */
+ currentBundleVersion?: string | null;
+
+ /**
+ * DAG-level configuration from the DAG details.
+ * If defined (true or false), takes precedence over global config.
+ */
+ dagLevelConfig?: boolean | null;
+
+ /**
+ * DAG run bundle version
+ */
+ runBundleVersion?: string | null;
+};
+
+type UseRunOnLatestVersionResult = {
+ /**
+ * Setter for the checkbox value
+ */
+ setValue: (value: boolean) => void;
+
+ /**
+ * Whether the checkbox should be shown
+ */
+ shouldShowCheckbox: boolean;
+
+ /**
+ * Current value of the checkbox (controlled)
+ */
+ value: boolean;
+};
+
+/**
+ * Custom hook for managing "Run on Latest Version" checkbox state.
+ *
+ * Implements the three-level precedence hierarchy:
+ * 1. DAG-level configuration (highest priority)
+ * 2. Global configuration
+ * 3. System default (false)
+ *
+ * Uses nullable override pattern: state is null until user interacts,
+ * avoiding the useState + useEffect synchronization anti-pattern.
+ */
+export const useRunOnLatestVersion = ({
+ currentBundleVersion,
+ dagLevelConfig,
+ runBundleVersion,
+}: UseRunOnLatestVersionProps): UseRunOnLatestVersionResult => {
+ const globalConfigValue = useConfig("run_on_latest_version");
+ const globalDefault = Boolean(globalConfigValue);
+
+ // Precedence: DAG-level > Global > System default (false)
+ const defaultValue = dagLevelConfig ?? globalDefault;
+
+ // Nullable override: null until user interacts, then stores their choice
+ const [userOverride, setUserOverride] = useState<boolean | null>(null);
+ const value = userOverride ?? defaultValue;
+
+ // Show checkbox only when versions differ and run has a valid bundle version
+ const hasValidRunVersion =
+ runBundleVersion !== undefined && runBundleVersion !== null &&
runBundleVersion !== "";
+ const shouldShowCheckbox = hasValidRunVersion && currentBundleVersion !==
runBundleVersion;
Review Comment:
As written, the checkbox will render when `currentBundleVersion` is still
`undefined` (e.g. DAG details not loaded yet) because `undefined !==
runBundleVersion` is `true`. This can cause the checkbox to flash or show with
an incorrect comparison. Require a valid `currentBundleVersion` (non-empty
string) before comparing, or delay showing until DAG details have loaded.
```suggestion
// Show checkbox only when both versions are valid and differ
const hasValidRunVersion =
runBundleVersion !== undefined && runBundleVersion !== null &&
runBundleVersion !== "";
const hasValidCurrentVersion =
currentBundleVersion !== undefined &&
currentBundleVersion !== null &&
currentBundleVersion !== "";
const shouldShowCheckbox =
hasValidRunVersion && hasValidCurrentVersion && currentBundleVersion !==
runBundleVersion;
```
##########
airflow-core/src/airflow/serialization/definitions/dag.py:
##########
@@ -1097,6 +1099,116 @@ def get_edge_info(self, upstream_task_id: str,
downstream_task_id: str) -> EdgeI
return empty
+def _resolve_bundle_version(
+ dag: SerializedDAG,
+ session: Session,
+) -> str | None:
+ """
+ Resolve the bundle version for a DAG run based on the precedence hierarchy.
+
+ Precedence (highest to lowest):
+ 1. DAG-level configuration (dag.run_on_latest_version)
+ 2. Global configuration (core.run_on_latest_version)
+ 3. System default (use original version from DagModel)
+
+ :param dag: The serialized DAG
+ :param session: Database session
+ :return: The resolved bundle version, or None if versioning is disabled
+ """
+ if dag.disable_bundle_versioning:
+ return None
+
+ # Determine whether to use latest version based on precedence hierarchy
+ use_latest = _should_use_latest_version(dag)
+ source = _get_config_source(dag)
+
+ if use_latest:
+ log.debug("Using latest bundle version", dag_id=dag.dag_id,
source=source)
+ return _get_latest_bundle_version(dag.dag_id, session)
+
+ log.debug("Using original bundle version", dag_id=dag.dag_id,
source=source)
Review Comment:
These `log.debug(..., dag_id=..., source=...)` / `log.warning(...,
dag_id=...)` calls pass arbitrary keyword arguments, which will raise
`TypeError` with standard `logging.Logger`. Use `%s` formatting args or
`extra={...}` instead (consistent with other logging in this file that uses
printf-style args).
```suggestion
log.debug("Using latest bundle version: dag_id=%s, source=%s",
dag.dag_id, source)
return _get_latest_bundle_version(dag.dag_id, session)
log.debug("Using original bundle version: dag_id=%s, source=%s",
dag.dag_id, source)
```
--
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]
