8000 fix(jobs): keep omitted settings unchanged on jobs/update by avison9 · Pull Request #69 · dmux/minilake · GitHub
[go: up one dir, main page]
Skip to content

fix(jobs): keep omitted settings unchanged on jobs/update - #69

Merged
dmux merged 3 commits into
dmux:mainfrom
avison9:fix/job-update-preserves-name-52
Oct 31, 2026
Merged

fix(jobs): keep omitted settings unchanged on jobs/update#69
dmux merged 3 commits into
dmux:mainfrom
avison9:fix/job-update-preserves-name-52

Conversation

@avison9
@avison9 avison9 commented Oct 26, 2026
Copy link
Copy Markdown
Contributor

Closes #52

Summary

A partial jobs/update that omits name renamed the job to Untitled. update_job merged a full
dump of new_settings into the stored settings, and that dump carries model defaults for fields the
caller never sent. Any SDK or Terraform workflow that edits only tags, timeouts or other settings
silently lost the job's name.

Before / after

from databricks.sdk import WorkspaceClient
from databricks.sdk.service.jobs import JobSettings

w = WorkspaceClient(host="http://localhost:8000", token="x")
job = w.jobs.create(name="valuable name", tasks=[])
w.jobs.update(job_id=job.job_id, new_settings=JobSettings(tags={"team": "qa"}))
print(w.jobs.get(job_id=job.job_id).settings.name)

# BEFORE: Untitled        <- the JobSettings default replaced the stored name
# AFTER:  valuable name

Root cause

JobSettings.name defaults to "Untitled", and update_job used
new_settings.model_dump(exclude_none=True), which includes defaults for omitted fields, so the
default overwrote the stored name on every update that did not send one.

What changed

update_job now merges only the top-level fields the request actually sent
(model_dump(exclude_none=True, include=new_settings.model_fields_set)). model_fields_set also
covers the extra="allow" fields, so settings such as email_notifications still merge.

The issue suggests exclude_unset=True. That was considered and not used: it also strips nested
defaults, so a task sent through update would lose the run_if: ALL_SUCCESS that create_job
stores, and updated jobs would be stored in a different shape from created ones. Filtering only the
top level keeps the stored shape identical. reset_job is unchanged, since a reset replaces the
whole settings object by design.

Testing

Full suite via docker-compose.test.yml, image rebuilt:

  • Before: 406 passed, 0 failed, 0 skipped
  • After: 410 passed, 0 failed, 0 skipped (the 4 new tests)
  • pre-commit run --all-files clean

New tests in tests/test_jobs.py, using the real databricks-sdk:

  • test_job_partial_update_keeps_omitted_fields[tags-only|timeout-only|empty]: the three cases
    listed in the issue. Each asserts the name and tasks survive and the sent field is applied.
  • test_job_update_with_name_renames: a name that is sent still replaces the stored one, so the fix
    does not turn update into a no-op for name.

Risk

Low. Only jobs/update is affected, and only fields the caller did not send behave differently:
they are now left alone instead of being reset to model defaults. Fields that are sent merge exactly
as before.

@avison9
fix(jobs): keep omitted settings unchanged on jobs/update
bf5e722 
update_job merged a full dump of new_settings, which carries model
defaults for fields the caller never sent, so a tags-only update reset
the job name to "Untitled". Merge only the top-level fields present in
the request. Nested defaults such as a task's run_if are kept, so
updated jobs are stored in the same shape as created ones.

Closes dmux#52
@dmux dmux self-assigned this Oct 29, 2026
@dmux dmux added the bug Something isn't working label Oct 29, 2026
@dmux
dmux commented Oct 29, 2026
Copy link
Copy Markdown
Owner

Hey @avison9, nice catch on this one! 🙌

It conflicts with main now: commit 1921949 changed update_job to validate a copy before saving. Can you rebase and move your model_fields_set change onto the new new_settings.update(...) line?

Also, a quick test with an extra field (like email_notifications) would be great.

Thanks!

@avison9
Merge upstream/main into fix/job-update-preserves-name-52
73f7a29 
Resolve the conflict with 1921949, which made update_job validate a copy of
the settings before saving: the sent-fields-only merge now applies to that
copy (new_settings), so a rejected update still leaves the job untouched.

Add a test with a setting the model doesn't declare (email_notifications):
kept when an update omits it, replaced when one sends it.
@avison9
avison9 commented Oct 31, 2026
Copy link
Copy Markdown
Contributor Author

Thanks @dmux. Updated:

  • Merged main and moved the change onto the new copy, so it's now new_settings.update(req.new_settings.model_dump(exclude_none=True, include=sent)). An update rejected by the task-graph validation still leaves the stored job untouched.
  • Added test_job_partial_update_keeps_and_replaces_extra_fields. It creates a job with email_notifications (not declared on JobSettings, passed through extra = "allow"). A tags-only update keeps both the name and email_notifications, and an update that sends email_notifications replaces it while the name and tags stay.

I merged rather than rebased to avoid force-pushing over the branch; happy to squash on merge if you prefer a single commit.

Full suite in Docker (docker-compose.test.yml): 542 passed, against 537 on main; the 5 extra are this PR's tests.

@dmux
dmux merged commit 1b0348e into dmux:main Oct 31, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@dmux dmux

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Updating only job tags resets the job name to Untitled
< 37CB div class="participation">

2 participants

@avison9 @dmux
0