Repository navigation
Send defaulted union tags in request bodies - #42
Open
vincent0426 wants to merge 1 commit into
Open
vincent0426 wants to merge 1 commit into
vincent0426 wants to merge 1 commit into
Conversation
The spec gives each tagged-union member's tag field (kind/typ) a
default, so the generated models default it too. Request bodies are
dumped with exclude_unset, which dropped a tag the caller left at its
default, and the server rejects the body:
400 Unable to extract tag using discriminator 'kind'
[type=union_tag_not_found]
For example, CreateModelRequest(source=ModelArchiveSource(name=...,
deployment=...)) sent no kind. Postprocessing now marks each defaulted
tag as set on every instance, so it is always sent, matching the Go
SDK's From* helpers. Other defaulted fields are still left to the
server.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the change preserving defaulted union discriminator tags during serialization.
No open issues 🎉
What changed in this PR
Fixes Python SDK requests that omit defaulted union discriminator tags, causing server-side validation errors. The change preserves these tags during serialization while leaving unrelated unset defaults omitted.
Changes:
- Update model postprocessing to mark defaulted discriminator fields as set.
- Regenerate management API and model configuration models with initialization hooks.
- Add regression tests for request serialization and validated model field tracking.
| File | Description |
|---|---|
tests/client/test_management.py |
Tests default tag inclusion and preservation of other unset defaults. |
scripts/apigen/postprocess.py |
Generates initialization hooks that mark defaulted discriminator fields as set. |
baseten/client/modelconfig/_models.py |
Adds hooks to protocol option models. |
baseten/client/managementapi/_models.py |
Adds hooks to tagged management API models. |
Review effort: Balanced
This branch has not been deployed
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
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.
🚀 What
Fixes requests failing with a 400 when a model uses the default
kindvalue. For example:fails with:
Today it only works if the caller passes
kind="model_archive"explicitly, even though the model already defaults to it.Go SDK always sends the tag, and JS types require it, so only Python is affected.
💻 How
Request bodies are dumped with
exclude_unset, so a tag left at its default was dropped. Postprocessing now marks each defaulted tag as set on every instance, so it's always sent.🔬 Testing
test_defaulted_union_tag_sent, which checks the request body includeskind, andtest_defaulted_union_tag_set_on_validated_response. Both fail without thischange.
poe lintandpoe typecheckare clean.A model-archive push requires the s3_key of an uploaded archive), which is expected.Note
The spec marks the tag optional with a default, but the server requires it. Happy to close this in favor of a spec/server fix if thats better.
The spec marks the tag optional with a default, but the server requires it. Happy to close this in favor of a spec/server fix if that's better.
https://github.com/basetenlabs/baseten-python/blob/3f1347cecbae0fa01244f9437a3ac5d4ac7746b2/scripts/apigen/specs/management.json#L23076-L23120