guan404ming commented on code in PR #61063:
URL: https://github.com/apache/airflow/pull/61063#discussion_r2746685741
##########
airflow-core/src/airflow/ui/src/components/Clear/Run/ClearRunDialog.tsx:
##########
@@ -45,13 +46,33 @@ const ClearRunDialog = ({ dagRun, onClose, open }: Props)
=> {
const [note, setNote] = useState<string | null>(dagRun.note);
const [selectedOptions, setSelectedOptions] =
useState<Array<string>>(["existingTasks"]);
const onlyFailed = selectedOptions.includes("onlyFailed");
- const [runOnLatestVersion, setRunOnLatestVersion] = useState(false);
// Get current DAG's bundle version to compare with DAG run's bundle version
const { data: dagDetails } = useDagServiceGetDagDetails({
dagId,
});
+ // Get global config for run_on_latest_version
+ const globalDefaultRunOnLatestVersion =
Boolean(useConfig("run_on_latest_version"));
+
+ // Determine checkbox default based on precedence: DAG-level > Global config
> System default (false)
+ const defaultRunOnLatestVersion = useMemo(() => {
Review Comment:
a single null-check has near-zero cost. Replace with nullish coalescing:
`dagDetails?.run_on_latest_version ?? globalDefault`
##########
airflow-core/src/airflow/ui/src/components/Clear/Run/ClearRunDialog.tsx:
##########
@@ -45,13 +46,33 @@ const ClearRunDialog = ({ dagRun, onClose, open }: Props)
=> {
const [note, setNote] = useState<string | null>(dagRun.note);
const [selectedOptions, setSelectedOptions] =
useState<Array<string>>(["existingTasks"]);
const onlyFailed = selectedOptions.includes("onlyFailed");
- const [runOnLatestVersion, setRunOnLatestVersion] = useState(false);
// Get current DAG's bundle version to compare with DAG run's bundle version
const { data: dagDetails } = useDagServiceGetDagDetails({
dagId,
});
+ // Get global config for run_on_latest_version
+ const globalDefaultRunOnLatestVersion =
Boolean(useConfig("run_on_latest_version"));
+
+ // Determine checkbox default based on precedence: DAG-level > Global config
> System default (false)
+ const defaultRunOnLatestVersion = useMemo(() => {
+ // Level 1: DAG-level configuration
+ if (dagDetails?.run_on_latest_version !== undefined &&
dagDetails.run_on_latest_version !== null) {
+ return dagDetails.run_on_latest_version;
+ }
+
+ // Level 2: Global configuration (Boolean() always returns true or false)
+ return globalDefaultRunOnLatestVersion;
+ }, [dagDetails?.run_on_latest_version, globalDefaultRunOnLatestVersion]);
+
+ const [runOnLatestVersion, setRunOnLatestVersion] =
useState(defaultRunOnLatestVersion);
Review Comment:
useState + useEffect to sync derived state will cause an extra re-render.
Use a nullable override pattern instead: keep override as null until the user
interacts, then resolve via `override ?? defaultValue`.
##########
airflow-core/src/airflow/ui/src/components/Clear/Run/ClearRunDialog.tsx:
##########
@@ -45,13 +46,33 @@ const ClearRunDialog = ({ dagRun, onClose, open }: Props)
=> {
const [note, setNote] = useState<string | null>(dagRun.note);
const [selectedOptions, setSelectedOptions] =
useState<Array<string>>(["existingTasks"]);
const onlyFailed = selectedOptions.includes("onlyFailed");
- const [runOnLatestVersion, setRunOnLatestVersion] = useState(false);
// Get current DAG's bundle version to compare with DAG run's bundle version
const { data: dagDetails } = useDagServiceGetDagDetails({
dagId,
});
+ // Get global config for run_on_latest_version
+ const globalDefaultRunOnLatestVersion =
Boolean(useConfig("run_on_latest_version"));
Review Comment:
Boolean("false") evaluates to true since any non-empty string is truthy. Use
explicit comparison (raw === "true") instead.
--
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]
