Skip to content

direct: auto-generate RemapState copiers from resource types - #6777

Draft
denik wants to merge 9 commits into
mainfrom
denik/remapstate-identity-comment
Draft

denik wants to merge 9 commits into
mainfrom
denik/remapstate-identity-comment

Conversation

@denik

@denik denik commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Most resources' RemapState is a mechanical copy of same-named fields from the remote type into the state type, filtering ForceSendFields. This compiles a copier per (remote, state) type pair at package init — driven off SupportedResources, so every auto-copied resource is validated at load, not as silent drift at deploy time — and wires it into the adapter when a resource has no custom RemapState. Deletes 12 redundant RemapState methods; resources with genuine remap logic keep theirs.

This pull request and its description were written by Isaac.

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 084d1de

Run: 36871287576

Env ✅​pass 🙈​skip Time
✅​ aws linux 276 15 5:32
✅​ aws windows 278 13 6:07
✅​ azure linux 275 15 5:51
✅​ azure windows 277 13 7:16
✅​ gcp linux 276 15 5:40
✅​ gcp windows 278 13 4:40
Top 6 slowest tests (at least 2 minutes):
duration env testname
7:12 azure windows TestAccept
6:03 aws windows TestAccept
4:38 gcp windows TestAccept
4:21 azure linux TestAccept
4:10 aws linux TestAccept
4:08 gcp linux TestAccept

@denik denik closed this Sep 22, 2026
@denik denik changed the title dresources: document why RemapState is skipped when types match direct: auto-generate RemapState copiers from resource types Sep 22, 2026
@denik denik reopened this Sep 22, 2026
@denik
denik force-pushed the denik/remapstate-identity-comment branch from 9ed6196 to ded8289 Compare September 23, 2026 10:03
@github-actions github-actions Bot added the DABs DABs related issues label Sep 23, 2026
@denik
denik force-pushed the denik/remapstate-identity-comment branch 2 times, most recently from 4a44ba8 to da7978d Compare September 29, 2026 12:18
@denik
denik force-pushed the denik/remapstate-identity-comment branch from da7978d to 5a565ba Compare September 30, 2026 10:14
denik added a commit that referenced this pull request Oct 1, 2026
`ResourceSecret`'s `RemapState` mapped `value`/`owner` from the API's
`effective_*` fields — needed so the persisted state matches config and
re-plans show no drift — but it also zeroed the output-only metadata
(`create_time`, `effective_value`, `metastore_id`, …), dropping those
fields from the plan's `remote_state` display.

This moves the `value`/`owner` mapping into a small in-place
`remapSecretRemote` applied in `DoRead`/`DoCreate`/`DoUpdate`, and stops
zeroing the rest: the output-only fields are already
`ignore_remote_changes`, so they cause no drift and can flow through for
visibility. `RemapState` is removed. Prep toward replacing hand-written
`RemapState` with the automatic copier (#6777).

This pull request and its description were written by Isaac.

---------

Co-authored-by: Isaac <no-reply@databricks.com>
denik added a commit that referenced this pull request Oct 1, 2026
…6812)

Surface `ForceSendFields` at the `ModelServingEndpointRemote` root and
marshal via the SDK's `marshal` package, so the force-send convention is
honored on the wrapper and `RemapState` becomes a plain subset copy —
prep for the automatic copier (#6777). Only the per-engine `*.direct.*`
plan goldens change; no action or diff changes.

This pull request and its description were written by Isaac.

Co-authored-by: Isaac <no-reply@databricks.com>
denik and others added 9 commits October 1, 2026 15:40
…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>
…er handles them)

After the prep PRs made these dumb, the auto-generated copier reproduces them
exactly (TestNoRedundantRemapState), so remove the methods.

Co-authored-by: Isaac <no-reply@databricks.com>
Removes RemapState for apps, jobs, job_runs, pipelines, the postgres_* resources
and quality_monitors - all plain subset copies the auto-generated copier
reproduces. Only model_serving_endpoints, vector_search_indexes and secrets keep
a RemapState for now; they need their own prep before the copier can take them,
after which this PR (rebased) will delete them and remove the RemapState hook.

Co-authored-by: Isaac <no-reply@databricks.com>
#6812 surfaced ForceSendFields at the ModelServingEndpointRemote root, making its
RemapState a plain copy. The auto-copier now reproduces it exactly
(TestNoRedundantRemapState), so remove the method and let buildCopiers handle it.

Co-authored-by: Isaac <no-reply@databricks.com>
@denik
denik force-pushed the denik/remapstate-identity-comment branch from 5a565ba to 084d1de Compare October 1, 2026 13:46

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

DABs DABs related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants