Repository: apache/airflow
Pull request: Adds validation on backfill dryrun
Adds some date checks before triggering backfill dryrun
Fixes: #47698

<!--
 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.
 -->

<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.

Feel free to ping committers for the review!

In case of an existing issue, reference it using one of the following:

closes: #ISSUE
related: #ISSUE

How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->



<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).


Problem statement:
AIP-38 Add validation to Run Backfill modal
### Body

On the Run Backfill modal. We should add a few more checks before allowiing a user to submit.

- Max Active Runs: Form should have a min of 1 and a max of dag.max_active_runs. We should default the user to 1
- Date range from and to should both be valid dates
- One reprocessor behavior should be selected. We should default to "Missing Runs"

### Committer

- [x] I acknowledge that I am a maintainer/committer of the Apache Airflow project.

Hints from issue discussion:
@bbovenzi currently we only show backfill count from dry run. It would be good if we can show dates as well.
@vatsrahul1001 we already have the date being selected in the form? So is there a need to re-display the same value? 
I'm also confused. @vatsrahul1001 could you explain more?
@bbovenzi currently from dry_run endpoint we are getting count I think here right? Would it make sense to show the exact dates on which backfill will be executed.

![Image](https://github.com/user-attachments/assets/b2d10d79-d167-4b3d-9aab-ba914e613a01)
Oh I see, like showing a table of the runs like we do with Clearing a dag run?

Commit message: Review comments handled

Patch to review:
diff --git a/airflow/ui/src/components/DagActions/RunBackfillForm.tsx b/airflow/ui/src/components/DagActions/RunBackfillForm.tsx
index a5da47640d5a0..c696efcf1e9a1 100644
--- a/airflow/ui/src/components/DagActions/RunBackfillForm.tsx
+++ b/airflow/ui/src/components/DagActions/RunBackfillForm.tsx
@@ -45,11 +45,11 @@ const RunBackfillForm = ({ dag, onClose }: RunBackfillFormProps) => {
     defaultValues: {
       dag_id: dag.dag_id,
       dag_run_conf: {},
-      from_date: undefined,
-      max_active_runs: undefined,
-      reprocess_behavior: undefined,
-      run_backwards: undefined,
-      to_date: undefined,
+      from_date: "",
+      max_active_runs: 1,
+      reprocess_behavior: "failed",
+      run_backwards: false,
+      to_date: "",
     },
     mode: "onBlur",
   });
@@ -62,11 +62,11 @@ const RunBackfillForm = ({ dag, onClose }: RunBackfillFormProps) => {
       requestBody: {
         dag_id: dag.dag_id,
         dag_run_conf: undefined,
-        from_date: values.from_date ?? today,
-        max_active_runs: values.max_active_runs ?? 0,
+        from_date: values.from_date ?? "",
+        max_active_runs: values.max_active_runs ?? 1,
         reprocess_behavior: values.reprocess_behavior,
         run_backwards: values.run_backwards ?? false,
-        to_date: values.to_date ?? today,
+        to_date: values.to_date ?? "",
       },
     },
   });
@@ -197,15 +197,22 @@ const RunBackfillForm = ({ dag, onClose }: RunBackfillFormProps) => {
           name="max_active_runs"
           render={({ field }) => (
             <HStack>
-              <Input {...field} placeholder="" type="number" width={24} />
+              <Input
+                {...field}
+                max={dag.max_active_runs ?? undefined}
+                min={1}
+                placeholder=""
+                type="number"
+                width={24}
+              />
               <Flex>Max Active Runs</Flex>
             </HStack>
           )}
         />
         <Spacer />
-        {affectedTasks.total_entries > 0 ? (
-          <Alert>{affectedTasks.total_entries} runs will be triggered</Alert>
-        ) : undefined}
+        <Alert visibility={affectedTasks.total_entries > 0 ? "visible" : "hidden"}>
+          {affectedTasks.total_entries} runs will be triggered
+        </Alert>
       </VStack>
       {dag.is_paused ? (
         <Checkbox checked={unpause} colorPalette="blue" onChange={() => setUnpause(!unpause)}>
diff --git a/airflow/ui/src/queries/useCreateBackfill.ts b/airflow/ui/src/queries/useCreateBackfill.ts
index 9c045f3b32735..79c65df3a02bc 100644
--- a/airflow/ui/src/queries/useCreateBackfill.ts
+++ b/airflow/ui/src/queries/useCreateBackfill.ts
@@ -50,7 +50,7 @@ export const useCreateBackfill = ({ onSuccessConfirm }: { onSuccessConfirm: () =
     const dataIntervalEnd = new Date(data.requestBody.to_date);
     const dagId = data.requestBody.dag_id;
 
-    if (!Boolean(dataIntervalStart) || !Boolean(dataIntervalEnd)) {
+    if (isNaN(dataIntervalStart.getTime()) || isNaN(dataIntervalEnd.getTime())) {
       setDateValidationError({
         body: {
           detail: "Both Data Interval Start Date and End Date must be provided.",
diff --git a/airflow/ui/src/queries/useCreateBackfillDryRun.ts b/airflow/ui/src/queries/useCreateBackfillDryRun.ts
index 8c6e7c9dd518a..3a615da968e67 100644
--- a/airflow/ui/src/queries/useCreateBackfillDryRun.ts
+++ b/airflow/ui/src/queries/useCreateBackfillDryRun.ts
@@ -28,12 +28,30 @@ type Props<TData, TError> = {
 
 const useCreateBackfillDryRunKey = "useCreateBackfillDryRunKey";
 
+const validateHeaderName = (requestBody: CreateBackfillDryRunData) => {
+  const dataIntervalStart = new Date(requestBody.requestBody.from_date);
+  const dataIntervalEnd = new Date(requestBody.requestBody.to_date);
+
+  if (isNaN(dataIntervalStart.getTime()) || isNaN(dataIntervalEnd.getTime())) {
+    return false;
+  }
+  if (dataIntervalStart > dataIntervalEnd) {
+    return false;
+  }
+  if (Number(requestBody.requestBody.max_active_runs ?? 0) < 1) {
+    return false;
+  }
+
+  return true;
+};
+
 export const useCreateBackfillDryRun = <TData = DryRunBackfillCollectionResponse, TError = unknown>({
   options,
   requestBody,
 }: Props<TData, TError>) =>
   useQuery<TData, TError>({
     ...options,
+    enabled: validateHeaderName(requestBody),
     queryFn: () =>
       BackfillService.createBackfillDryRun({
         ...requestBody,


Code review comments:
diff_hunk: @@ -50,7 +50,7 @@ export const useCreateBackfill = ({ onSuccessConfirm }: { onSuccessConfirm: () =
     const dataIntervalEnd = new Date(data.requestBody.to_date);
     const dagId = data.requestBody.dag_id;
 
-    if (!Boolean(dataIntervalStart) || !Boolean(dataIntervalEnd)) {
+    if (isNaN(dataIntervalStart.getTime()) || isNaN(dataIntervalEnd.getTime())) {
path: airflow/ui/src/queries/useCreateBackfill.ts
text: @user1:
Is this required ? `!Boolean` check seems clearer to me that calling `getTime` and checking for `isNaN`

Otherwise checking explicitely for `null` or `undefined` is better. 

@author:
Hi, so the output for date for a malformed string was a date object with invalid date, which when passed to Boolean would return true. So had to do it this way... 

@user1:
How can `dataIntervalStart` or `dataIntervalEnd` be malformed string ? (Date constructor as well as the input field should guarantee that we are manipulating a valid date / datetime I think, I might be missing something)

Is that to handle the case where `from_date` or `to_date` are `date` instead of `datetime` ?

@author:
When I mention malformed here it can mostly only be either a date or an empty string (When no date is selected at start time), so I can maybe change the check to just a not empty string check...

@author:
Done