Add performance_target to Databricks RunNow and CreateJobs operators - #72148
Conversation
a8e6e4a to
2f02c4f
Compare
|
cc @li-xiang-db @moomindani for Databricks review |
There was a problem hiding this comment.
LGTM — approving. I checked this from both sides, since "the API accepts the same field" is the kind of claim that is easy to state and easy to get subtly wrong.
Databricks side, verified against a live workspace:
jobs/createacceptsperformance_targetat the top level andjobs/getechoes it back insettings— so the placement in_get_named_json_parametersis right forDatabricksCreateJobsOperator.- The run-level override the
DatabricksRunNowOperatordocstring claims is real, not just accepted: a job created withperformance_target: STANDARD, triggered viarun-nowwithPERFORMANCE_OPTIMIZED, produced a run whoseeffective_performance_targetisPERFORMANCE_OPTIMIZED. The run object carrieseffective_performance_target, notperformance_target, which is worth knowing if anyone later wants to surface it. - A job whose tasks use classic
new_clustercompute is not rejected whenperformance_targetis set — it is stored as given. So the operators do not need to gate the field on serverless, and the docstrings' "on serverless compute" wording is the right level of caution.
Airflow side:
performance_targetis intemplate_fieldsfor both operators, which matchesDatabricksSubmitRunOperatorwhere #71374 added it — all three surfaces now template it identically.- Merge semantics are the same as every other named parameter:
_merge_json_with_named_parametersdropsNonevalues and lets the named argument win over a key already injson, so passing nothing changes no payload and passing both is resolved the same way asidempotency_tokenand friends. - "This field will be templated." matches the phrasing already used for the neighbouring parameters in that file.
- The two new tests each assert the merged payload and would fail without the change.
One thing worth a line in the docstring (non-blocking, details inline): an unrecognised value is accepted with 200 and dropped, and the run then falls back to the default — which is the more expensive mode. Measured: with no field set the run's effective_performance_target is PERFORMANCE_OPTIMIZED; with "STANDARD" it is STANDARD; with a typo like "STANDART" it is PERFORMANCE_OPTIMIZED again. So mistyping the cost-saving value costs money silently, with no error from Airflow or from the API.
That asymmetry is why I raise it for this parameter rather than as a general remark. The other silently-tolerant fields are cheaper to get wrong — a dropped edit_mode leaves the job editable, an invalid schedule.pause_status becomes UNPAUSED and shows up in run history — while several fields in the same payload validate properly (git_source.git_provider lists its accepted values; access_control_list[].permission_level, health.rules.metric and max_concurrent_runs are rejected outright).
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.
The fourth surface, DatabricksWorkflowTaskGroup, has no performance_target parameter, but it takes extra_job_params, which overrides default job properties — so it is reachable there without adding another argument. Nothing to do, just noting it so the gap is not mistaken for one.
Drafted-by: Claude Code (Opus 5); reviewed by @moomindani before posting
Signed-off-by: PoAn Yang <payang@apache.org>
2f02c4f to
a1c8fa9
Compare
|
The docstring is what I was after, and covering all three operators rather than just the one I commented on is better than what I asked for. Diffed my approved commit Drafted-by: Claude Code (Opus 5); reviewed by @moomindani before posting |
…pache#72148) Signed-off-by: PoAn Yang <payang@apache.org>
#71374 added
performance_targettoDatabricksSubmitRunOperator. Thejobs/createandjobs/run-nowendpoints accept the same field, but the two operators that call them still lack it, so these three operators now behave differently for one API field. This exposes it as a templated named parameter that merges into the request like the other task fields.Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.