Lee-W commented on code in PR #62393:
URL: https://github.com/apache/airflow/pull/62393#discussion_r2844214395


##########
providers/databricks/tests/unit/databricks/operators/test_databricks.py:
##########
@@ -1634,6 +1634,39 @@ def test_no_cancel_previous_runs(self, db_mock_class):
         db_mock.get_run_page_url.assert_called_once_with(RUN_ID)
         db_mock.get_run.assert_not_called()
 
+    
@mock.patch("airflow.providers.databricks.operators.databricks.DatabricksHook")
+    def test_cancel_previous_runs_without_job_id_raises(self, db_mock_class):
+        run = {
+            "notebook_params": NOTEBOOK_PARAMS,
+            "notebook_task": NOTEBOOK_TASK,
+            "jar_params": JAR_PARAMS,
+        }
+
+        op = DatabricksRunNowOperator(
+            task_id=TASK_ID,
+            json=run,
+            cancel_previous_runs=True,
+        )
+
+        db_mock = db_mock_class.return_value
+
+        with pytest.raises(
+            ValueError,
+            match="cancel_previous_runs=True requires either job_id or 
job_name",
+        ):
+            op.execute(None)
+
+        db_mock_class.assert_called_once_with(
+            DEFAULT_CONN_ID,
+            retry_limit=op.databricks_retry_limit,
+            retry_delay=op.databricks_retry_delay,
+            retry_args=None,
+            caller="DatabricksRunNowOperator",
+        )
+
+        db_mock.cancel_all_runs.assert_not_called()
+        db_mock.run_now.assert_not_called()

Review Comment:
   ```suggestion
           assert db_mock_class.mock_calls == [
               call(
                   DEFAULT_CONN_ID,
                   retry_limit=op.databricks_retry_limit,
                   retry_delay=op.databricks_retry_delay,
                   retry_args=None,
                   caller="DatabricksRunNowOperator",
               )
           ]
           assert db_mock.cancel_all_runs.mock_calls == []
           assert db_mock.run_now.mock_calls == []
   ```
   



##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -921,8 +921,15 @@ def execute(self, context: Context):
             self.json["job_id"] = job_id
             del self.json["job_name"]
 
-        if self.cancel_previous_runs and self.json["job_id"] is not None:
-            hook.cancel_all_runs(self.json["job_id"])
+        if self.cancel_previous_runs:
+            job_id = self.json.get("job_id")
+
+            if job_id is None:
+                raise ValueError(
+                    "cancel_previous_runs=True requires either job_id or 
job_name to be provided."
+                )

Review Comment:
   ```suggestion
               if (job_id := self.json.get("job_id")) is None:
                   raise ValueError(
                       "cancel_previous_runs=True requires either job_id or 
job_name to be provided."
                   )
   ```
   
   nit



-- 
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]

Reply via email to