direct: raise resource generator recursion cap from 4 to 10 - #6376
Merged
Conversation
The generator's field-behavior walker capped recursion at depth 4. Now that
fields legitimately sit at depth 5 -- e.g.
config.served_entities[*].external_model.custom_provider_config.{api_key_auth.value_plaintext,bearer_token_auth.token_plaintext},
where the extra auth wrapper object pushes them one level past the cap while
the sibling providers' plaintext keys land at depth 4 and were already caught.
The cap is not needed for termination: `visited` already expands each schema
type at most once, so recursion is bounded regardless. Output saturates at
depth 5 (cap 5, 6, and 10 produce identical results), so 10 is pure headroom
as a runaway guard.
Regenerate resources.generated.yml (adds the two fields above) and drop the
now-redundant manual workaround for them in resources.yml.
Co-authored-by: Isaac <no-reply@databricks.com>
radakam
reviewed
Aug 25, 2026
| reason: spec:input_only | ||
| - field: config.served_entities[*].external_model.cohere_config.cohere_api_key_plaintext | ||
| reason: spec:input_only | ||
| - field: config.served_entities[*].external_model.custom_provider_config.api_key_auth.value_plaintext |
Contributor
There was a problem hiding this comment.
should we also add a test covering custom_provider_config?
Contributor
Author
There was a problem hiding this comment.
I've extended acceptance/bundle/resources/model_serving_endpoints/drift/write_only to verify it works e2e. But in general we probably need a test on the generate_resources.py tool to assert behaviour (at arbitrary depth). Leaving this out of scope of this PR though.
Collaborator
Integration test reportCommit: 09dab5c
7 interesting tests: 4 SKIP, 2 flaky, 1 RECOVERED
Top 3 slowest tests (at least 2 minutes):
|
andrewnester
approved these changes
Aug 25, 2026
radakam
approved these changes
Aug 25, 2026
radakam
left a comment
radakam
left a comment
Contributor
There was a problem hiding this comment.
probably makes sense to add .nextchanges?
denik
approved these changes
Aug 25, 2026
| # (e.g. external_model.custom_provider_config.bearer_token_auth.token_plaintext), | ||
| # so keep ample headroom above that. | ||
| if depth > 10: | ||
| return {} |
Contributor
There was a problem hiding this comment.
should we crash here instead of silently dropping fields?
Contributor
Author
There was a problem hiding this comment.
this is build-time so yes, let's crash and raise the limit if we encounter resources nested that deep
Contributor
Author
@radakam not a user-facing change since the field was already marked input-only, this is internal-only |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Why
#6366 allowed field behaviour from nested fields, but
custom_provider_configfields were still skipped.The cap of depth 4 was arbitrary and 10 is sufficient to prevent runaway.
Tests
Existing ones pass, added
custom_provider_configto exercise it e2eBefore bumping max_depth to 10 the
task generate-direct-resourceswould fail like this: