Skip to content

direct: drop redundant RemapState methods handled by the copier - #6793

Draft
denik wants to merge 8 commits into
denik/remapstate-identity-commentfrom
denik/drop-redundant-remapstate
Draft

denik wants to merge 8 commits into
denik/remapstate-identity-commentfrom
denik/drop-redundant-remapstate

Conversation

@denik

@denik denik commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Stacked on #6777. Now that the framework auto-copies remote→state, 15 RemapState methods are dead boilerplate the copier reproduces exactly:

  • apps, clusters, sql_warehouses recomputed a lifecycle.started value that DoRead already stores; the copier carries DoRead's value through. For non-running/terminated transient states this now follows DoRead's mapping (nil), which is the authoritative one per the README.
  • jobs, job_runs, pipelines, model_serving_endpoints, quality_monitors, and the seven postgres_* resources were plain field-by-field subset copies.

buildCopiers validates each at package init, so anything that couldn't be copied safely would fail at load. Verified: unit suite (incl. the init guard) and acceptance tests for the affected resources pass.

This pull request and its description were written by Isaac.

denik and others added 7 commits September 21, 2026 16:59
…s state type

Co-authored-by: Isaac <no-reply@databricks.com>
Resources whose state type is a plain subset of the remote type had a
hand-written RemapState that mechanically copied same-named fields and
filtered ForceSendFields. Replace those with a copier compiled once from
the (remote, state) type pair at package init and wired into the adapter
when a resource has no custom RemapState.

buildCopiers is driven off SupportedResources, so every auto-copied
resource is validated at load: anything the copier cannot copy losslessly
(kind-changing conversions, mismatched struct shapes) fails there rather
than as silent drift at deploy time. Deletes 12 redundant RemapState
methods; resources with genuine logic keep theirs.

Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Drop the duplicate reflection filler in favor of the existing fillNonZero,
populating ForceSendFields with a small dedicated helper. Field-per-line
table in TestSafeConvert; shorten the README section.

Co-authored-by: Isaac <no-reply@databricks.com>
Move the reflection copier (compile-time type check, field/JSON-name copy,
ForceSendFields filtering, lossless conversions) out of dresources into a
reusable package beside structwalk/structdiff/structaccess. The direct engine
wiring (buildCopiers, driven off SupportedResources) stays in dresources and
now calls structcopy.Compile.

Co-authored-by: Isaac <no-reply@databricks.com>
The auto-generated copier reproduces these 15 RemapState methods exactly, so
delete them and let buildCopiers handle the copy:

- apps, clusters, sql_warehouses recomputed a lifecycle value DoRead already
  stores; the copier now carries DoRead's value through (the authoritative one).
- jobs, job_runs, pipelines, model_serving_endpoints, quality_monitors and the
  postgres_* resources were plain field-by-field subset copies.

Co-authored-by: Isaac <no-reply@databricks.com>
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 3d66375

Run: 35726925541

Env ✅​pass 🙈​skip Time
✅​ aws linux 276 15 6:35
✅​ aws windows 278 13 7:32
✅​ azure linux 275 15 4:59
✅​ azure windows 277 13 4:06
✅​ gcp linux 276 15 5:45
✅​ gcp windows 278 13 4:17
Top 3 slowest tests (at least 2 minutes):
duration env testname
7:28 aws windows TestAccept
4:14 gcp windows TestAccept
4:03 azure windows TestAccept

Its ForceSendFields come from a nested field (EndpointDetails.ForceSendFields),
which the copier (root ForceSendFields only) cannot reproduce, so the auto-copy
diverges. Acceptance caught it on the direct engine; keep the override.

Co-authored-by: Isaac <no-reply@databricks.com>
@denik
denik marked this pull request as draft September 22, 2026 13:13
@denik
denik force-pushed the denik/remapstate-identity-comment branch from 9ed6196 to ded8289 Compare September 23, 2026 10:03

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants