You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ha2hi (via GitHub)" <gi...@apache.org> on 2026/04/06 14:55:42 UTC
[PR] Add DatabricksDeleteJobOperator to Databricks provider [airflow]
ha2hi opened a new pull request, #64766:
URL: https://github.com/apache/airflow/pull/64766
### Description
Currently, the Databricks provider lacks an operator to delete an existing Databricks job, although the Databricks REST API supports this functionality.
This PR introduces the `DatabricksDeleteJobsOperator` to fill this gap.
**Changes made:**
1. **DatabricksHook:** Added a `delete_job(self, job_id: int)` method to interact with the `api/2.2/jobs/delete` endpoint.
2. **DatabricksDeleteJobsOperator:** Created the operator using the updated hook.
3. **Unit Tests:** Added comprehensive unit tests in `test_databricks.py` to verify the operator's execution, correct hook initialization with retry arguments, and template fields.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4346764806
cc @mwojtyczka @alexott can you take a look?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3066486590
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,51 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int,
+ databricks_conn_id: str = "databricks_default",
Review Comment:
`job_id` is marked as a templated field, but the constructor type is `int`. In Airflow, Jinja rendering typically produces `str` values, so this signature is inconsistent with templating and with other Databricks operators (e.g. `DatabricksRunNowOperator` uses `job_id: str | None`). Consider widening the type (e.g. `int | str`) and/or coercing to `int` in `execute` before calling the hook.
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,51 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
Review Comment:
The class docstring documents only `job_id` and `databricks_conn_id`, but `__init__` also exposes retry-related parameters (and `polling_period_seconds`). Please document these parameters here for consistency with other operators in this module (e.g. `DatabricksCreateJobsOperator`).
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -302,6 +303,14 @@ def create_job(self, json: dict) -> int:
response = self._do_api_call(CREATE_ENDPOINT, json)
return response["job_id"]
+ def delete_job(self, job_id: int) -> None:
+ """
+ Call the ``api/2.2/jobs/delete`` endpoint.
+
+ :param job_id: The unique identifier of the job to be deleted.
+ """
+ self._do_api_call(DELETE_ENDPOINT, {"job_id": job_id})
Review Comment:
`delete_job` takes `job_id: int`, but operator/template usage commonly yields a `str` at runtime. Consider accepting `str | int` (consistent with `reset_job`/`update_job` signatures) and/or coercing/validating to an `int` before making the API call to ensure the payload type matches the Databricks API expectation.
##########
providers/databricks/tests/unit/databricks/operators/test_databricks.py:
##########
@@ -603,6 +604,44 @@ def test_exec_update_job_permission_with_empty_acl(self, db_mock_class):
db_mock.update_job_permission.assert_not_called()
+class TestDatabricksDeleteJobsOperator:
+ @mock.patch("airflow.providers.databricks.operators.databricks.DatabricksHook")
+ def test_execute(self, mock_hook_class):
+ # Set up the mock hook instance
+ mock_hook = mock_hook_class.return_value
+
+ # Instantiate the DatabricksDeleteJobOperator
+ op = DatabricksDeleteJobsOperator(
+ task_id="delete_job_task",
+ job_id=12345,
+ databricks_conn_id="databricks_default",
+ )
+
+ # Execute the operator
+ op.execute({})
+
+ # Verify that the DatabricksHook was initialized with the correct connection ID and default retry arguments
+ mock_hook_class.assert_called_once_with(
+ "databricks_default",
+ retry_limit=3,
+ retry_delay=1,
+ retry_args=None,
Review Comment:
This assertion hard-codes default retry values (`retry_limit=3`, `retry_delay=1`). For consistency with the other operator tests in this file and to avoid brittleness if defaults change (or if the operator is instantiated with non-default values), assert against `op.databricks_retry_limit` / `op.databricks_retry_delay` instead of literals.
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,51 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int,
+ databricks_conn_id: str = "databricks_default",
+ polling_period_seconds: int = 30,
+ databricks_retry_limit: int = 3,
+ databricks_retry_delay: int = 1,
+ databricks_retry_args: dict[Any, Any] | None = None,
+ **kwargs,
+ ) -> None:
+ super().__init__(**kwargs)
+ self.job_id = job_id
+ self.databricks_conn_id = databricks_conn_id
+ self.polling_period_seconds = polling_period_seconds
+ self.databricks_retry_limit = databricks_retry_limit
+ self.databricks_retry_delay = databricks_retry_delay
+ self.databricks_retry_args = databricks_retry_args
Review Comment:
`polling_period_seconds` is accepted and stored on the operator, but it is never used by `DatabricksDeleteJobsOperator`. This makes the public API misleading; consider removing the parameter (and attribute) or using it if there is intended polling behavior.
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -54,6 +54,7 @@
REPAIR_RUN_ENDPOINT = ("POST", "2.2/jobs/runs/repair")
OUTPUT_RUNS_JOB_ENDPOINT = ("GET", "2.2/jobs/runs/get-output")
CANCEL_ALL_RUNS_ENDPOINT = ("POST", "2.2/jobs/runs/cancel-all")
+DELETE_ENDPOINT = ("POST", "2.2/jobs/delete")
Review Comment:
Endpoint constant name `DELETE_ENDPOINT` is overly generic in this module, which already defines other delete-related endpoints like `DELETE_RUN_ENDPOINT` and `DELETE_REPO_ENDPOINT`. Renaming this to something specific like `DELETE_JOB_ENDPOINT` would better match existing naming patterns and avoid confusion.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "Zinkue (via GitHub)" <gi...@apache.org>.
Zinkue commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4364436385
Thanks for the answer @ha2hi!
That is an interesting use case and this operator can help the clean-up of leftovers from Airflow.
My personal preference will be to keep jobs synced between environments.
In any case, since I am familiarized with Databricks, I can also review it from a functional perspective 😊
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "MZC-Garnett (via GitHub)" <gi...@apache.org>.
MZC-Garnett commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4206378395
@potiuk
I have resolved the pre-commit checks, successfully rebased with the main branch, and removed the unnecessary local test file (dags/test_dag.py).
The PR is now ready for review! Thank you for your patience and guidance.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3068085621
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -54,6 +54,7 @@
REPAIR_RUN_ENDPOINT = ("POST", "2.2/jobs/runs/repair")
OUTPUT_RUNS_JOB_ENDPOINT = ("GET", "2.2/jobs/runs/get-output")
CANCEL_ALL_RUNS_ENDPOINT = ("POST", "2.2/jobs/runs/cancel-all")
+DELETE_ENDPOINT = ("POST", "2.2/jobs/delete")
Review Comment:
I intentionally named it DELETE_ENDPOINT to maintain consistency with existing Job-related endpoints in this file, such as CREATE_ENDPOINT, RESET_ENDPOINT, and UPDATE_ENDPOINT, which do not include the JOB infix.
To resolve this, I think we have three options:
1. **Change only this new endpoint** : Rename it to DELETE_JOB_ENDPOINT as Copilot suggested.
2. **Rename all job endpoints** : Add the JOB infix to all existing legacy endpoints (e.g., CREATE_JOB_ENDPOINT, RESET_JOB_ENDPOINT) for overall clarity.
3. **Maintain current convention** : Keep it as DELETE_ENDPOINT to match the existing names in this module.
I lean towards Option 3 to avoid scope creep or potential breaking changes if anyone is importing those constants directly, but I'm fully open to whichever direction the maintainers prefer. Please let me know what you think!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4593376395
@ha2hi This draft PR has been inactive for 7 days since the last triage comment and no response from the author. Closing to keep the queue clean.
You are welcome to reopen this PR when you resume work, or to open a new one addressing the issues previously raised. There is no rush — take your time.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4414763698
Hi, @Zinkue
Would you like to check the changed code?
1. Add docs
2. Add find_job_id_by_name method
3. Added exception handling for cases where job_id or job_name is missing
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4206387454
@potiuk
I have resolved the pre-commit checks, successfully rebased with the main branch, and removed the unnecessary local test file (dags/test_dag.py).
The PR is now ready for review! Thank you for your patience and guidance.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4300310304
Quick follow-up to the triage comment above — one clarification on the "Unresolved review comments" item:
Once you believe a thread has been addressed — whether by pushing a fix, or by replying in-thread with an explanation of why the suggestion doesn't apply — **please mark the thread as resolved yourself** by clicking the "Resolve conversation" button at the bottom of each thread. Reviewers don't auto-close their own threads, so an addressed-but-unresolved thread reads as "still waiting on the author" and keeps the PR from moving forward. The author doing the resolve-click is the expected convention on this project.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3147381666
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,51 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int,
+ databricks_conn_id: str = "databricks_default",
Review Comment:
Good catch. I will widen the type of job_id to int | str
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #64766: Add DatabricksDeleteJobsOperator to Databricks provider
URL: https://github.com/apache/airflow/pull/64766
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3147388709
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,51 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
Review Comment:
Thanks for pointing this out. I'll update the docstring to include the missing parameters as suggested to maintain consistency.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4529613791
@ha2hi A few things need addressing before review — see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
- :x: **CI fails**: `CI image checks / Build documentation (--docs-only)` (and possibly other checks — see the [Checks tab](https://github.com/apache/airflow/pull/64766/checks) for the full list).
No rush.
---
_Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this [two-stage triage process](https://github.com/apache/airflow/blob/main/contributing-docs/25_maintainer_pr_triage.md#why-the-first-pass-is-automated) so that our maintainers' limited time is spent where it matters most: the conversation with you._
---
Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "Zinkue (via GitHub)" <gi...@apache.org>.
Zinkue commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4359097755
Out of curiosity, what is the reason to add this operator? Any specific use case?
From my experience using this provider the existing operators can handle one-time runs or updates of existing jobs
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3214518809
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,54 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ :param databricks_retry_limit: Amount of times retry if the Databricks backend is unreachable.
+ Its value must be greater than or equal to 1.
+ :param databricks_retry_delay: Number of seconds to wait between retries (it
+ might be a floating point number).
+ :param databricks_retry_args: An optional dictionary with arguments passed to ``tenacity.Retrying``.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int | str,
+ databricks_conn_id: str = "databricks_default",
+ databricks_retry_limit: int = 3,
+ databricks_retry_delay: int = 1,
+ databricks_retry_args: dict[Any, Any] | None = None,
+ **kwargs,
+ ) -> None:
+ super().__init__(**kwargs)
+ self.job_id = job_id
+ self.databricks_conn_id = databricks_conn_id
+ self.databricks_retry_limit = databricks_retry_limit
+ self.databricks_retry_delay = databricks_retry_delay
+ self.databricks_retry_args = databricks_retry_args
+
+ @cached_property
+ def _hook(self):
+ return DatabricksHook(
+ self.databricks_conn_id,
+ retry_limit=self.databricks_retry_limit,
+ retry_delay=self.databricks_retry_delay,
+ retry_args=self.databricks_retry_args,
+ caller="DatabricksDeleteJobsOperator",
+ )
+
+ def execute(self, context: Context) -> None:
+ self._hook.delete_job(self.job_id)
+
+ self.log.info("Successfully deleted Databricks job ID: %s", self.job_id)
Review Comment:
I will add a mechanism to raise an error if the job_id does not exist.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3147413233
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,51 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int,
+ databricks_conn_id: str = "databricks_default",
+ polling_period_seconds: int = 30,
+ databricks_retry_limit: int = 3,
+ databricks_retry_delay: int = 1,
+ databricks_retry_args: dict[Any, Any] | None = None,
+ **kwargs,
+ ) -> None:
+ super().__init__(**kwargs)
+ self.job_id = job_id
+ self.databricks_conn_id = databricks_conn_id
+ self.polling_period_seconds = polling_period_seconds
+ self.databricks_retry_limit = databricks_retry_limit
+ self.databricks_retry_delay = databricks_retry_delay
+ self.databricks_retry_args = databricks_retry_args
Review Comment:
Since polling_period_seconds is not utilized within this operator, I've removed the parameter and the attribute to avoid misleading the users.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3147400755
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -302,6 +303,14 @@ def create_job(self, json: dict) -> int:
response = self._do_api_call(CREATE_ENDPOINT, json)
return response["job_id"]
+ def delete_job(self, job_id: int) -> None:
+ """
+ Call the ``api/2.2/jobs/delete`` endpoint.
+
+ :param job_id: The unique identifier of the job to be deleted.
+ """
+ self._do_api_call(DELETE_ENDPOINT, {"job_id": job_id})
Review Comment:
I've updated the signature to accept str | int for consistency with other methods and added explicit coercion to int before the API call to ensure the payload type is correct.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3192098687
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,54 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ :param databricks_retry_limit: Amount of times retry if the Databricks backend is unreachable.
+ Its value must be greater than or equal to 1.
+ :param databricks_retry_delay: Number of seconds to wait between retries (it
+ might be a floating point number).
+ :param databricks_retry_args: An optional dictionary with arguments passed to ``tenacity.Retrying``.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int | str,
+ databricks_conn_id: str = "databricks_default",
+ databricks_retry_limit: int = 3,
+ databricks_retry_delay: int = 1,
+ databricks_retry_args: dict[Any, Any] | None = None,
+ **kwargs,
+ ) -> None:
+ super().__init__(**kwargs)
+ self.job_id = job_id
+ self.databricks_conn_id = databricks_conn_id
+ self.databricks_retry_limit = databricks_retry_limit
+ self.databricks_retry_delay = databricks_retry_delay
+ self.databricks_retry_args = databricks_retry_args
+
+ @cached_property
+ def _hook(self):
+ return DatabricksHook(
+ self.databricks_conn_id,
+ retry_limit=self.databricks_retry_limit,
+ retry_delay=self.databricks_retry_delay,
+ retry_args=self.databricks_retry_args,
+ caller="DatabricksDeleteJobsOperator",
+ )
+
+ def execute(self, context: Context) -> None:
+ self._hook.delete_job(self.job_id)
Review Comment:
Thanks for the suggestion.
I’ll incorporate it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4195341975
@ha2hi This PR has a few issues that need to be addressed before it can be reviewed — please see our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Pre-commit / static checks**: Failing: CI image checks / Static checks. Run `prek run --from-ref main` locally to find and fix issues. See [Pre-commit / static checks docs](https://github.com/apache/airflow/blob/main/contributing-docs/08_static_code_checks.rst).
> **Note:** Your branch is **69 commits behind `main`**. Some check failures may be caused by changes in the base branch rather than by your PR. Please rebase your branch and push again to get up-to-date CI results.
**What to do next:**
- The comment informs you what you need to do.
- Fix each issue, then mark the PR as "Ready for review" in the GitHub UI - but only after making sure that all the issues are fixed.
- There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates.
- Maintainers will then proceed with a normal review.
There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4298824750
@ha2hi Thanks for your earlier reply — there are still six unresolved threads that need your attention. This PR has been converted to **draft** because it does not yet meet our [Pull Request quality criteria](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-quality-criteria).
**Issues found:**
- :x: **Unresolved review comments** (6 threads): please walk through each unresolved review thread. **Even if a suggestion looks incorrect or irrelevant** — and some of them will be, especially any comments left by automated reviewers like GitHub Copilot — it is still the author's responsibility to respond: apply the fix, reply in-thread with a brief explanation of why the suggestion does not apply, or resolve the thread if the feedback is no longer relevant. Leaving threads unaddressed for weeks blocks the PR from moving forward.
**What to do next:**
- Walk through each unresolved review thread and respond as described above.
- Make sure static checks pass locally: `prek run --from-ref main --stage pre-commit`.
- Mark the PR as "Ready for review" when you're done.
Converting a PR to draft is **not** a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the [Airflow Slack](https://s.apache.org/airflow-slack).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "MZC-Garnett (via GitHub)" <gi...@apache.org>.
MZC-Garnett commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3068082406
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -54,6 +54,7 @@
REPAIR_RUN_ENDPOINT = ("POST", "2.2/jobs/runs/repair")
OUTPUT_RUNS_JOB_ENDPOINT = ("GET", "2.2/jobs/runs/get-output")
CANCEL_ALL_RUNS_ENDPOINT = ("POST", "2.2/jobs/runs/cancel-all")
+DELETE_ENDPOINT = ("POST", "2.2/jobs/delete")
Review Comment:
I intentionally named it DELETE_ENDPOINT to maintain consistency with existing Job-related endpoints in this file, such as CREATE_ENDPOINT, RESET_ENDPOINT, and UPDATE_ENDPOINT, which do not include the _JOB_ infix.
To resolve this, I think we have three options:
1. **Change only this new endpoint** : Rename it to DELETE_JOB_ENDPOINT as Copilot suggested.
2. **Rename all job endpoints** : Add the _JOB_ infix to all existing legacy endpoints (e.g., CREATE_JOB_ENDPOINT, RESET_JOB_ENDPOINT) for overall clarity.
3. **Maintain current convention** : Keep it as DELETE_ENDPOINT to match the existing names in this module.
I lean towards Option 3 to avoid scope creep or potential breaking changes if anyone is importing those constants directly, but I'm fully open to whichever direction the maintainers prefer. Please let me know what you think!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "MZC-Garnett (via GitHub)" <gi...@apache.org>.
MZC-Garnett commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3068082406
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -54,6 +54,7 @@
REPAIR_RUN_ENDPOINT = ("POST", "2.2/jobs/runs/repair")
OUTPUT_RUNS_JOB_ENDPOINT = ("GET", "2.2/jobs/runs/get-output")
CANCEL_ALL_RUNS_ENDPOINT = ("POST", "2.2/jobs/runs/cancel-all")
+DELETE_ENDPOINT = ("POST", "2.2/jobs/delete")
Review Comment:
I intentionally named it DELETE_ENDPOINT to maintain consistency with existing Job-related endpoints in this file, such as CREATE_ENDPOINT, RESET_ENDPOINT, and UPDATE_ENDPOINT, which do not include the _JOB_ infix.
To resolve this, I think we have three options:
1. **Change only this new endpoint** : Rename it to DELETE_JOB_ENDPOINT as Copilot suggested.
2. **Rename all job endpoints** : Add the _JOB_ infix to all existing legacy endpoints (e.g., CREATE_JOB_ENDPOINT, RESET_JOB_ENDPOINT) for overall clarity.
3. **Maintain current convention** : Keep it as DELETE_ENDPOINT to match the existing names in this module.
I lean towards Option 3 to avoid scope creep or potential breaking changes if anyone is importing those constants directly, but I'm fully open to whichever direction the maintainers prefer. Please let me know what you think!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3147406494
##########
providers/databricks/tests/unit/databricks/operators/test_databricks.py:
##########
@@ -603,6 +604,44 @@ def test_exec_update_job_permission_with_empty_acl(self, db_mock_class):
db_mock.update_job_permission.assert_not_called()
+class TestDatabricksDeleteJobsOperator:
+ @mock.patch("airflow.providers.databricks.operators.databricks.DatabricksHook")
+ def test_execute(self, mock_hook_class):
+ # Set up the mock hook instance
+ mock_hook = mock_hook_class.return_value
+
+ # Instantiate the DatabricksDeleteJobOperator
+ op = DatabricksDeleteJobsOperator(
+ task_id="delete_job_task",
+ job_id=12345,
+ databricks_conn_id="databricks_default",
+ )
+
+ # Execute the operator
+ op.execute({})
+
+ # Verify that the DatabricksHook was initialized with the correct connection ID and default retry arguments
+ mock_hook_class.assert_called_once_with(
+ "databricks_default",
+ retry_limit=3,
+ retry_delay=1,
+ retry_args=None,
Review Comment:
Good point. I'll replace the hard-coded literals with operator attributes (e.g., op.databricks_retry_limit) to avoid test brittleness and maintain consistency.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3068085621
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -54,6 +54,7 @@
REPAIR_RUN_ENDPOINT = ("POST", "2.2/jobs/runs/repair")
OUTPUT_RUNS_JOB_ENDPOINT = ("GET", "2.2/jobs/runs/get-output")
CANCEL_ALL_RUNS_ENDPOINT = ("POST", "2.2/jobs/runs/cancel-all")
+DELETE_ENDPOINT = ("POST", "2.2/jobs/delete")
Review Comment:
I intentionally named it DELETE_ENDPOINT to maintain consistency with existing Job-related endpoints in this file, such as CREATE_ENDPOINT, RESET_ENDPOINT, and UPDATE_ENDPOINT, which do not include the JOB infix.
(https://github.com/apache/airflow/blob/511af0a7dd559063c82800fb21b56e4f1c008316/providers/databricks/src/airflow/providers/databricks/hooks/databricks.py#L46-L48)
To resolve this, I think we have three options:
1. **Change only this new endpoint** : Rename it to DELETE_JOB_ENDPOINT as Copilot suggested.
2. **Rename all job endpoints** : Add the JOB infix to all existing legacy endpoints (e.g., CREATE_JOB_ENDPOINT, RESET_JOB_ENDPOINT) for overall clarity.
3. **Maintain current convention** : Keep it as DELETE_ENDPOINT to match the existing names in this module.
I lean towards Option 3 to avoid scope creep or potential breaking changes if anyone is importing those constants directly, but I'm fully open to whichever direction the maintainers prefer. Please let me know what you think!
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks.py:
##########
@@ -54,6 +54,7 @@
REPAIR_RUN_ENDPOINT = ("POST", "2.2/jobs/runs/repair")
OUTPUT_RUNS_JOB_ENDPOINT = ("GET", "2.2/jobs/runs/get-output")
CANCEL_ALL_RUNS_ENDPOINT = ("POST", "2.2/jobs/runs/cancel-all")
+DELETE_ENDPOINT = ("POST", "2.2/jobs/delete")
Review Comment:
I intentionally named it DELETE_ENDPOINT to maintain consistency with existing Job-related endpoints in this file, such as CREATE_ENDPOINT, RESET_ENDPOINT, and UPDATE_ENDPOINT, which do not include the JOB infix.
https://github.com/apache/airflow/blob/511af0a7dd559063c82800fb21b56e4f1c008316/providers/databricks/src/airflow/providers/databricks/hooks/databricks.py#L46-L48
To resolve this, I think we have three options:
1. **Change only this new endpoint** : Rename it to DELETE_JOB_ENDPOINT as Copilot suggested.
2. **Rename all job endpoints** : Add the JOB infix to all existing legacy endpoints (e.g., CREATE_JOB_ENDPOINT, RESET_JOB_ENDPOINT) for overall clarity.
3. **Maintain current convention** : Keep it as DELETE_ENDPOINT to match the existing names in this module.
I lean towards Option 3 to avoid scope creep or potential breaking changes if anyone is importing those constants directly, but I'm fully open to whichever direction the maintainers prefer. Please let me know what you think!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "Zinkue (via GitHub)" <gi...@apache.org>.
Zinkue commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3177054610
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,54 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ :param databricks_retry_limit: Amount of times retry if the Databricks backend is unreachable.
+ Its value must be greater than or equal to 1.
+ :param databricks_retry_delay: Number of seconds to wait between retries (it
+ might be a floating point number).
+ :param databricks_retry_args: An optional dictionary with arguments passed to ``tenacity.Retrying``.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int | str,
+ databricks_conn_id: str = "databricks_default",
+ databricks_retry_limit: int = 3,
+ databricks_retry_delay: int = 1,
+ databricks_retry_args: dict[Any, Any] | None = None,
+ **kwargs,
+ ) -> None:
+ super().__init__(**kwargs)
+ self.job_id = job_id
+ self.databricks_conn_id = databricks_conn_id
+ self.databricks_retry_limit = databricks_retry_limit
+ self.databricks_retry_delay = databricks_retry_delay
+ self.databricks_retry_args = databricks_retry_args
+
+ @cached_property
+ def _hook(self):
+ return DatabricksHook(
+ self.databricks_conn_id,
+ retry_limit=self.databricks_retry_limit,
+ retry_delay=self.databricks_retry_delay,
+ retry_args=self.databricks_retry_args,
+ caller="DatabricksDeleteJobsOperator",
+ )
+
+ def execute(self, context: Context) -> None:
+ self._hook.delete_job(self.job_id)
Review Comment:
I would recommend using the `find_job_id_by_name` method from the hook (see `DatabricksCreateJobsOperator.execute`)
You know before hand the job's name rather his id
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "Zinkue (via GitHub)" <gi...@apache.org>.
Zinkue commented on code in PR #64766:
URL: https://github.com/apache/airflow/pull/64766#discussion_r3177056320
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -410,6 +410,54 @@ def execute(self, context: Context) -> int:
return job_id
+class DatabricksDeleteJobsOperator(BaseOperator):
+ """
+ Deletes a Databricks job.
+
+ :param job_id: The ID of the Databricks job to delete. (templated)
+ :param databricks_conn_id: Reference to the Databricks connection.
+ :param databricks_retry_limit: Amount of times retry if the Databricks backend is unreachable.
+ Its value must be greater than or equal to 1.
+ :param databricks_retry_delay: Number of seconds to wait between retries (it
+ might be a floating point number).
+ :param databricks_retry_args: An optional dictionary with arguments passed to ``tenacity.Retrying``.
+ """
+
+ template_fields: Sequence[str] = ("job_id",)
+
+ def __init__(
+ self,
+ *,
+ job_id: int | str,
+ databricks_conn_id: str = "databricks_default",
+ databricks_retry_limit: int = 3,
+ databricks_retry_delay: int = 1,
+ databricks_retry_args: dict[Any, Any] | None = None,
+ **kwargs,
+ ) -> None:
+ super().__init__(**kwargs)
+ self.job_id = job_id
+ self.databricks_conn_id = databricks_conn_id
+ self.databricks_retry_limit = databricks_retry_limit
+ self.databricks_retry_delay = databricks_retry_delay
+ self.databricks_retry_args = databricks_retry_args
+
+ @cached_property
+ def _hook(self):
+ return DatabricksHook(
+ self.databricks_conn_id,
+ retry_limit=self.databricks_retry_limit,
+ retry_delay=self.databricks_retry_delay,
+ retry_args=self.databricks_retry_args,
+ caller="DatabricksDeleteJobsOperator",
+ )
+
+ def execute(self, context: Context) -> None:
+ self._hook.delete_job(self.job_id)
+
+ self.log.info("Successfully deleted Databricks job ID: %s", self.job_id)
Review Comment:
In case the job does not exist it should raise an exception like job not found
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "ha2hi (via GitHub)" <gi...@apache.org>.
ha2hi commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4362903701
Hi, @Zinkue
We create Jobs in a development workspace, deploy them to a production workspace via DAB(Databricks Asset Bundle), and then remove the development Jobs to avoid leaving unused Jobs behind.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add DatabricksDeleteJobsOperator to Databricks provider [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64766:
URL: https://github.com/apache/airflow/pull/64766#issuecomment-4415990957
> Hi, @Zinkue Would you like to check the changed code?
>
> 1. Add docs
> 2. Add find_job_id_by_name method
> 3. Added exception handling for cases where job_id or job_name is missing
I suggest you make the PR green first.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org