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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
