Skip to content

fix(BA-5983): make ModelConfig GQL input fields optional#11531

Draft
jopemachine wants to merge 6 commits intomainfrom
fix/BA-5983-modelconfig-fields-optional
Draft

fix(BA-5983): make ModelConfig GQL input fields optional#11531
jopemachine wants to merge 6 commits intomainfrom
fix/BA-5983-modelconfig-fields-optional

Conversation

@jopemachine
Copy link
Copy Markdown
Member

@jopemachine jopemachine commented May 8, 2026

Resolves BA-5983

Summary

  • addModelRevision rejected requests that omitted ModelConfigInput.name (and similarly model_path, ModelServiceConfigInput.port, ModelHealthCheckInput.path) with ModelConfig.name is required, even though those fields are routinely supplied by other layers in the revision merge chain — the runtime variant's default_model_definition, the model vfolder's model-definition.yaml, a revision preset, or the model_mount_destination default.
  • Re-bind ModelConfigInputGQL / ModelDefinitionInputGQL / ModelServiceConfigInputGQL / ModelHealthCheckInputGQL to the *Draft DTO variants from common/config so every field the merge chain can supply is nullable at the GQL boundary.
  • Required-field enforcement is unchanged: it still happens inside {ModelConfig,ModelHealthCheck,ModelServiceConfig}Draft.to_resolved() after the full merge in DeploymentController.add_revision().

Schema impact (input-only, non-breaking):

  • ModelConfigInput.{name, modelPath}: non-null → nullable
  • ModelServiceConfigInput.{port, shell, preStartActions}: non-null → nullable
  • ModelHealthCheckInput.{path, interval, maxRetries, maxWaitTime, expectedStatusCode, initialDelay}: non-null → nullable
  • ModelDefinitionInput.models: non-null → nullable
  • All output (ModelConfig, ModelDefinition, etc.) types untouched.

The supergraph / schema.graphql / v2-schema.graphql regeneration will be pushed by the update-api-schema workflow on this PR.

Test plan

  • addModelRevision succeeds when modelDefinition.models[].name / modelPath are omitted and the runtime variant or vfolder model-definition.yaml supplies them.
  • addModelRevision succeeds when modelDefinition is omitted entirely (variant baseline alone is sufficient).
  • Existing requests that DO supply name / modelPath continue to work unchanged.
  • addModelRevision still fails with a clear error when no layer supplies a required field (validation at to_resolved() is preserved).
  • update-api-schema workflow regenerates schema.graphql / v2-schema.graphql / supergraph.graphql cleanly.

🤖 Generated with Claude Code


📚 Documentation preview 📚: https://sorna--11531.org.readthedocs.build/en/11531/


📚 Documentation preview 📚: https://sorna-ko--11531.org.readthedocs.build/ko/11531/

The addModelRevision mutation rejected requests that omitted
ModelConfigInput.name (and similarly model_path, ModelServiceConfigInput.port,
ModelHealthCheckInput.path), even though those fields are routinely supplied
by other layers in the revision merge chain — the runtime variant's
default_model_definition, the model vfolder's model-definition.yaml, a
revision preset, or the model_mount_destination default.

Bind the input types (ModelConfigInputGQL, ModelDefinitionInputGQL,
ModelServiceConfigInputGQL, ModelHealthCheckInputGQL) to the *Draft DTO
variants from common/config so every field that the merge chain can
supply is nullable at the GQL boundary. Required-field validation stays
where it belongs — in {ModelConfig,ModelHealthCheck,ModelServiceConfig}Draft
.to_resolved() after the full merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the size:M 30~100 LoC label May 8, 2026
@jopemachine jopemachine added this to the 26.4 milestone May 8, 2026
@github-actions github-actions Bot added the comp:manager Related to Manager component label May 8, 2026
Co-authored-by: octodog <mu001@lablup.com>
@github-actions github-actions Bot added size:L 100~500 LoC area:docs Documentations and removed size:M 30~100 LoC labels May 8, 2026
Re-route the GraphQL/REST v2 input boundary through dedicated
``Model{HealthCheck,Metadata,ServiceConfig,Config,Definition}Input``
DTOs in ``common/dto/manager/v2/deployment/request.py`` instead of
binding directly to the merge-chain ``*Draft`` domain models from
``common/config``. The ``*Draft`` types remain the internal merge-chain
representation; the boundary types are owned by the v2 DTO package.

- New v2 Input DTOs mirror the structure of the corresponding
  ``*Draft`` types (every field optional) so the request layer stays
  permissive and lower-priority sources (runtime variant default,
  revision preset, vfolder ``model-definition.yaml``) can supply
  whatever the request omits.
- ``CreateRevisionInputDTO``/``AddRevisionGQLInputDTO``/v2
  ``RevisionInput`` now type ``model_definition`` as
  ``ModelDefinitionInput`` instead of leaking
  ``ModelDefinitionDraft`` from ``common/config`` into the v2 DTO
  package.
- Add ``to_model_definition_draft`` converter alongside the DTOs and
  call it at the GQL adapter boundary (``manager/api/adapters/
  deployment/adapter.py``) before constructing
  ``ModelRevisionCreator``; the legacy REST path is unchanged
  (still uses the deprecated ``common/dto/manager/deployment``
  ``RevisionInput``).
- Re-bind ``Model*InputGQL`` types in
  ``manager/api/gql/deployment/types/revision.py`` to the new v2
  Input DTOs and drop the temporary ``*DraftDTO`` aliases introduced
  in the previous commit.

Required-field validation still happens in
``ModelConfigDraft.to_resolved`` / ``ModelHealthCheckDraft.to_resolved``
/ ``ModelServiceConfigDraft.to_resolved`` after the merge, which is
called in ``DeploymentController.add_revision``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the comp:common Related to Common component label May 8, 2026
@jopemachine jopemachine removed this from the 26.4 milestone May 8, 2026
jopemachine and others added 2 commits May 8, 2026 20:02
Drop verbose per-field descriptions on the new v2 ModelConfig /
ModelDefinition / ModelServiceConfig / ModelHealthCheck Input DTOs and
their GQL counterparts where the field name is already self-describing
or the description merely repeated the merge-chain note. Keep one
class-level note on ModelDefinitionInput as the single canonical
explanation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentations comp:common Related to Common component comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant