moomindani commented on code in PR #69998:
URL: https://github.com/apache/airflow/pull/69998#discussion_r3835874904
##########
providers/databricks/src/airflow/providers/databricks/plugins/databricks_workflow.py:
##########
@@ -471,13 +499,38 @@ class WorkflowJobRepairSingleTaskLink(BaseOperatorLink,
LoggingMixin):
name = "Repair a single task"
+ @property
+ def operators(self):
+ # Declared so deserialization keeps this plugin link instead of
replacing it with
+ # XComOperatorLink. Lazy import avoids a circular import with the
operator module.
+ from airflow.providers.databricks.operators.databricks import (
+ DatabricksNotebookOperator,
+ DatabricksTaskOperator,
+ )
+
+ return [DatabricksNotebookOperator, DatabricksTaskOperator]
Review Comment:
This makes the plugin link match every `DatabricksNotebookOperator` /
`DatabricksTaskOperator`, and
plugin links take precedence over the operator's own serialized links ("If
OperatorLinks with the same
name exists, Links via Plugin have higher precedence",
`serialization/serialized_objects.py:1117-1120`).
That is what fixes the workflow case, but it also overrides
`operators/databricks.py:1690-1697`, where
the operator deliberately picks its link set per case:
```python
else:
# Databricks does not support repair for non-workflow tasks, hence do
not show the repair link.
self.operator_extra_links = (DatabricksJobRunLink(),)
```
On Airflow 3 that is harmless, and I measured it: a standalone
`DatabricksNotebookOperator` gains a
`"Repair a single task"` entry that resolves to `None` —
`_get_launch_task_id_v3` finds no launch task
and `get_link` returns `""` — and the UI drops null URLs, so only the
`/links` response gains a null key.
On Airflow 2 it is not harmless. The provider still declares
`apache-airflow>=2.11.0`
(`pyproject.toml:62`); 2.11's deserializer has the same plugin matching and
the same precedence merge
(`serialized_objects.py:1266-1272` and `:1310-1315`); and the pre-3.0 branch
of `get_link` degrades
differently from the 3.x branch above. For a top-level task
`operator.task_group` is the Dag's root
group, so the `if not task_group` guard does not fire, and
`get_launch_task_id` then raises
`AirflowException("No launch task can be found in the task group.")`.
Airflow 2.11's extra-links view
catches only `ValueError` (`airflow/www/views.py:3249-3252`), so this
surfaces as a 500 behind a button
the UI has already rendered — on the plain notebook operator, which is the
common case.
I cannot run 2.11 here, so that last chain is code reading rather than
measurement, but every step of it
is in the released 2.11 source. Suggested fix in the next comment.
`WorkflowJobRepairAllFailedLink` does not have this problem —
`_CreateDatabricksWorkflowOperator` only
ever exists inside a workflow task group.
---
Drafted-by: Claude Code (Opus 5); reviewed by @moomindani before posting
##########
providers/databricks/src/airflow/providers/databricks/plugins/databricks_workflow.py:
##########
@@ -471,13 +499,38 @@ class WorkflowJobRepairSingleTaskLink(BaseOperatorLink,
LoggingMixin):
name = "Repair a single task"
+ @property
+ def operators(self):
+ # Declared so deserialization keeps this plugin link instead of
replacing it with
+ # XComOperatorLink. Lazy import avoids a circular import with the
operator module.
+ from airflow.providers.databricks.operators.databricks import (
+ DatabricksNotebookOperator,
+ DatabricksTaskOperator,
+ )
+
+ return [DatabricksNotebookOperator, DatabricksTaskOperator]
+
def get_link( # type: ignore[override] # Signature intentionally kept
this way for Airflow 2.x compatibility
self,
operator,
dttm=None,
*,
ti_key: TaskInstanceKey | None = None,
) -> str:
+ if AIRFLOW_V_3_0_PLUS:
+ if not AIRFLOW_V_3_1_PLUS or ti_key is None:
+ # The Airflow-3 repair backend requires 3.1+ (see
DatabricksWorkflowPlugin).
+ return ""
+ launch_task_id = _get_launch_task_id_v3(operator, ti_key)
+ if not launch_task_id:
+ return ""
Review Comment:
This is the graceful degradation the Airflow 2 branch lacks. Now that the
link is attached to
non-workflow tasks as well, the cheapest fix is to mirror this shape below:
when the launch task cannot
be resolved, return `""` instead of letting `get_launch_task_id(task_group)`
raise — i.e. around
```python
if ".launch" not in ti_key.task_id:
launch_task_id = get_launch_task_id(task_group)
```
catch the "no launch task in this group" case and return `""`, so a
standalone Databricks task renders
no URL rather than a 500. That keeps the behaviour of the two branches
aligned and leaves the workflow
case untouched.
---
Drafted-by: Claude Code (Opus 5); reviewed by @moomindani 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
