Skip to content

Commit 7d762f1

Browse files
committed
fix(FR-2832): send null for definitionPath when not customized (#7315)
Resolves #7283 (FR-2832) ## Summary When creating a deployment revision, the WebUI was sending the literal string `"model-definition.yaml"` as `modelMountConfig.definitionPath`. This forced the backend to look for that exact filename and broke model folders that use `model-definition.yml`. After this change, the frontend **omits** `definitionPath` when the user has not explicitly customized the filename, so the backend can fall back to its own resolution logic and accept either extension. ## Changes (single file: `react/src/components/DeploymentAddRevisionModal.tsx`) 1. **`initialValues`** — dropped the `definitionPath: 'model-definition.yaml'` seed. The form field now starts empty; the existing `placeholder="model-definition.yaml"` on the Input still hints the default. 2. **Submit handler** — for the non-custom / non-command-mode branch, **omit** `definitionPath` from the `modelMountConfig` payload when the trimmed user input is empty. The `isCustom && isCommandMode` branch still sends the literal `'model-definition.yaml'` because we generate that file ourselves before submission. 3. **Edit-mode pre-population** — dropped the `?? 'model-definition.yaml'` fallback so when an existing revision had `definitionPath: null/empty`, the form leaves the field empty and a re-submit continues to omit the field. 4. **Required rule removed** — the `definitionPath` Form.Item no longer has `rules={[{ required: true }]}` so users can submit an empty value (which is now the explicit "use server default" signal). ## Backend dependency > **Requires backend to accept an absent / nullable `modelMountConfig.definitionPath` and resolve both `.yaml`/`.yml` server-side.** The current GraphQL schema declares `ModelMountConfigInput.definitionPath: String!` (non-null). The submit handler uses a conditional spread to omit the field when not customized; this works at runtime because most GraphQL backends treat an absent input field as "use default." A small `as { definitionPath: string }` cast on the spread is needed to satisfy the current strict type — flagged with `TODO(needs-backend): FR-2832`. The backend must relax the schema field to nullable and update the resolver to try `model-definition.yaml` then `model-definition.yml` (or vice versa) when this field is absent. ## Out of scope Other call sites of `'model-definition.yaml'` (`ServiceLauncherPageContent.tsx`, `LegacyModelTryContentButton.tsx`, `DeploymentLauncherPageContent.tsx`, `VFolderTextFileEditorModal.tsx`, `DeploymentLauncherPage.tsx`) are intentionally untouched per the issue triage. A follow-up issue tracks applying the same pattern to the launcher components. ## Verification `bash scripts/verify.sh`: - Relay: PASS - Lint: PASS - Format: PASS - TypeScript: only **pre-existing** errors remain (`packages/backend.ai-client/src/client.ts`, `src/components/DeleteForeverVFolderModalV2.tsx`) — confirmed identical against `main` before this change. ## Test plan - [ ] Create a new deployment revision **without** typing anything in the "Model Definition Path" field → mutation payload should **omit** `modelMountConfig.definitionPath`. - [ ] Create a new deployment revision with **whitespace only** (e.g. `" "`) → mutation payload should **omit** `modelMountConfig.definitionPath` (whitespace is trimmed). - [ ] Create a new deployment revision **with** an explicit value (e.g. `custom.yaml`) → mutation payload should send `modelMountConfig.definitionPath: "custom.yaml"`. - [ ] Edit an existing revision whose previous `definitionPath` is empty/null → form field stays empty, re-submit keeps omitting the field. - [ ] Edit an existing revision whose previous `definitionPath` is `"model-definition.yaml"` → form field shows that value, re-submit sends it back unchanged. - [ ] Custom variant + command mode → literal `"model-definition.yaml"` is still sent (since we generate that file ourselves). - [ ] Once backend lands its companion change, verify that a model folder containing only `model-definition.yml` deploys successfully via this UI. ## Review feedback applied - **Replaced `as string` cast with conditional spread.** The submit handler no longer lies to TypeScript by asserting `null` is a `string`. Instead, the field is omitted entirely from the input object when the user has not customized it (`...(resolvedDefinitionPath ? { definitionPath: resolvedDefinitionPath } : {})`). A narrower `as { definitionPath: string }` cast on the spread expression remains until the schema is relaxed to nullable. - **Trim whitespace before truthiness check.** `values.definitionPath?.trim()` is computed once into `trimmedDefinitionPath`, so whitespace-only inputs (`" "`) collapse to "not customized" and the backend default applies. - **Comment consolidation.** The two multi-paragraph FR-2832 comments now share a single `TODO(needs-backend)` block at the spread site; the resolver site has a one-line back-reference.
1 parent 004104b commit 7d762f1

2 files changed

Lines changed: 37 additions & 21 deletions

File tree

data/schema.graphql

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9235,10 +9235,10 @@ input ModelConfigInput
92359235
@join__type(graph: STRAWBERRY)
92369236
{
92379237
"""Name of the model."""
9238-
name: String!
9238+
name: String = null
92399239

92409240
"""Path to the model file."""
9241-
modelPath: String!
9241+
modelPath: String = null
92429242

92439243
"""Configuration for the model service."""
92449244
service: ModelServiceConfigInput = null
@@ -9264,7 +9264,7 @@ input ModelDefinitionInput
92649264
@join__type(graph: STRAWBERRY)
92659265
{
92669266
"""List of models in the model definition."""
9267-
models: [ModelConfigInput!]!
9267+
models: [ModelConfigInput!] = null
92689268
}
92699269

92709270
"""
@@ -9431,22 +9431,22 @@ input ModelHealthCheckInput
94319431
@join__type(graph: STRAWBERRY)
94329432
{
94339433
"""Interval in seconds between health checks."""
9434-
interval: Float! = 10
9434+
interval: Float = null
94359435

94369436
"""Path to check for health status."""
9437-
path: String!
9437+
path: String = null
94389438

94399439
"""Maximum number of retries for health check."""
9440-
maxRetries: Int! = 10
9440+
maxRetries: Int = null
94419441

94429442
"""Maximum time in seconds to wait for a health check response."""
9443-
maxWaitTime: Float! = 15
9443+
maxWaitTime: Float = null
94449444

94459445
"""Expected HTTP status code for a healthy response."""
9446-
expectedStatusCode: Int! = 200
9446+
expectedStatusCode: Int = null
94479447

94489448
"""Initial delay in seconds before the first health check."""
9449-
initialDelay: Float! = 60
9449+
initialDelay: Float = null
94509450
}
94519451

94529452
"""Added in 26.4.2. Metadata describing a model entry."""
@@ -9558,7 +9558,7 @@ input ModelMountConfigInput
95589558
{
95599559
vfolderId: ID!
95609560
mountDestination: String!
9561-
definitionPath: String!
9561+
definitionPath: String = null
95629562
}
95639563

95649564
"""
@@ -9808,16 +9808,16 @@ input ModelServiceConfigInput
98089808
"""
98099809
List of pre-start actions to execute before starting the model service.
98109810
"""
9811-
preStartActions: [PreStartActionInput!]!
9811+
preStartActions: [PreStartActionInput!] = null
98129812

98139813
"""Command to start the model service."""
98149814
startCommand: [String!] = null
98159815

98169816
"""Shell configured for the model service."""
9817-
shell: String! = "/bin/bash"
9817+
shell: String = null
98189818

9819-
"""Port number for the model service. Must be greater than 1."""
9820-
port: Int!
9819+
"""Port number for the model service."""
9820+
port: Int = null
98219821

98229822
"""Health check configuration for the model service."""
98239823
healthCheck: ModelHealthCheckInput = null

react/src/components/DeploymentAddRevisionModal.tsx

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -502,8 +502,11 @@ const DeploymentAddRevisionModalFormBody: React.FC<
502502
? rev.modelMountConfig.vfolderId.replace(/-/g, '')
503503
: undefined,
504504
mountDestination: rev.modelMountConfig?.mountDestination ?? '/models',
505-
definitionPath:
506-
rev.modelMountConfig?.definitionPath ?? 'model-definition.yaml',
505+
// FR-2832: don't fall back to `'model-definition.yaml'` here — when
506+
// the previous revision left `definitionPath` empty/null, keep the
507+
// form field empty so a re-submit continues to send `null` instead
508+
// of re-introducing the hardcoded default.
509+
definitionPath: rev.modelMountConfig?.definitionPath,
507510
// `ImageEnvironmentSelectFormItems` matches the form's
508511
// `environments.version` against its image catalog by canonical
509512
// name; setting this is enough to drive the rest of the
@@ -771,10 +774,15 @@ const DeploymentAddRevisionModalFormBody: React.FC<
771774
isCustom && isCommandMode
772775
? (values.commandModelMount ?? '/models')
773776
: values.mountDestination || '/models';
774-
const definitionPath =
777+
// FR-2832: see TODO at the modelMountConfig spread below for backend
778+
// coordination details. Trim user input so whitespace-only values
779+
// (`' '`) collapse to "not customized" and let the backend pick
780+
// the default filename.
781+
const trimmedDefinitionPath = values.definitionPath?.trim();
782+
const resolvedDefinitionPath =
775783
isCustom && isCommandMode
776784
? 'model-definition.yaml'
777-
: values.definitionPath || 'model-definition.yaml';
785+
: trimmedDefinitionPath || null;
778786

779787
onIsAddingChange(true);
780788
commitAdd({
@@ -804,7 +812,12 @@ const DeploymentAddRevisionModalFormBody: React.FC<
804812
// normalize before submitting (same as `extraMounts`).
805813
vfolderId: convertToUUID(values.modelFolderId),
806814
mountDestination,
807-
definitionPath,
815+
// FR-2832: send `null` for `definitionPath` when the user
816+
// hasn't customized it so the backend can resolve either
817+
// `model-definition.yaml` or `model-definition.yml`.
818+
// `ModelMountConfigInput.definitionPath` is nullable in the
819+
// schema (default `null`).
820+
definitionPath: resolvedDefinitionPath,
808821
},
809822
modelDefinition,
810823
extraMounts: extraMounts.length > 0 ? extraMounts : null,
@@ -888,7 +901,11 @@ const DeploymentAddRevisionModalFormBody: React.FC<
888901
onFinishFailed={handleFinishFailed}
889902
initialValues={_.merge({}, RESOURCE_ALLOCATION_INITIAL_FORM_VALUES, {
890903
mountDestination: '/models',
891-
definitionPath: 'model-definition.yaml',
904+
// FR-2832: don't seed `definitionPath` — leave it empty so the
905+
// submit handler can send `null` and let the backend resolve
906+
// either `model-definition.yaml` or `model-definition.yml`.
907+
// The Input below shows `model-definition.yaml` as a placeholder
908+
// so users still see the default hint.
892909
customDefinitionMode: 'command',
893910
commandPort: 8000,
894911
commandHealthCheck: '/health',
@@ -1083,7 +1100,6 @@ const DeploymentAddRevisionModalFormBody: React.FC<
10831100
<Form.Item
10841101
name="definitionPath"
10851102
label={t('deployment.ModelDefinitionPath')}
1086-
rules={[{ required: true }]}
10871103
style={{ flex: 1 }}
10881104
>
10891105
<Input allowClear placeholder="model-definition.yaml" />

0 commit comments

Comments
 (0)