moomindani commented on code in PR #72148:
URL: https://github.com/apache/airflow/pull/72148#discussion_r3887844298
##########
providers/databricks/src/airflow/providers/databricks/operators/databricks.py:
##########
@@ -452,6 +452,13 @@ class DatabricksCreateJobsOperator(BaseOperator):
.. seealso::
This will only be used on create. In order to reset ACL consider
using the Databricks
UI.
+ :param performance_target: Optional performance mode for runs of this job
on serverless compute.
+ Either ``PERFORMANCE_OPTIMIZED`` (prioritizes fast startup and
execution) or
+ ``STANDARD`` (enables cost-efficient execution of serverless
workloads). This field
+ will be templated.
+
+ .. seealso::
+ https://docs.databricks.com/api/workspace/jobs/create
Review Comment:
Suggestion: one sentence here about the default, because the way this field
fails is asymmetric.
Measured against a live workspace — `jobs/create`, then `run-now`, reading
`effective_performance_target` off the resulting run:
| `performance_target` sent | effective target of the run |
| --- | --- |
| no field | `PERFORMANCE_OPTIMIZED` |
| `"STANDARD"` | `STANDARD` |
| `"STANDART"` (typo) | `PERFORMANCE_OPTIMIZED` |
An unrecognised value is accepted with 200 and dropped — `jobs/get`
afterwards shows no `performance_target` at all — so the run falls back to the
default, which is the *more expensive* mode. Someone who sets `STANDARD` to get
cost-efficient serverless and mistypes it gets no error from Airflow or from
the API; the only trace is `effective_performance_target` on the run.
That earns a line here in a way it would not for the other silently-tolerant
fields: a dropped `edit_mode` just leaves the job editable, and an invalid
`schedule.pause_status` comes back as `UNPAUSED`, which shows up in run
history. Here nothing looks wrong and only the bill changes. Other fields in
the same payload do validate (`git_source.git_provider` answers with its list
of accepted values; `access_control_list[].permission_level` and
`max_concurrent_runs` are rejected outright), so the tolerance is per-field
rather than a contract to rely on.
Concretely, something like: "Values other than these two are dropped by the
API rather than rejected, and the run then uses the default —
`PERFORMANCE_OPTIMIZED` on the workspace I tested."
Not a request for client-side validation: that was settled on #71374, where
the reasoning was that existing workloads pass unrecognised values and run on
the default, so hard-erroring would break them and the API side is being fixed.
Whether the API should coerce silently at all belongs to that thread, not this
PR.
---
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]
