Skip to content

Commit 6a09225

Browse files
xvnyvxinyitan
andauthored
fix: correct mlserver uri conversion regardless of filenames (#43)
#### Motivation As documented in [this issue](kserve#40), there is currently a bug in the conversion of the `uri` provided in the MLServer `model-settings.json` file if the model files are symlinked before the uri conversion occurs. This PR is intended to fix the issue by ensuring that `model-settings.json` will always be processed first. #### Modifications The function `adaptModelLayoutForRuntime` in the MLServer adapter `server.go` was modified to ensure that, if present, the `model-settings.json` file will be placed at the head of the list of files to be processed. This makes sure that the config file will always be processed before any symlinks are created for the model files. A unit test was also added to ensure that the bug was fixed. #### Result Incorrect uri conversion bug no longer occurs even when the model files alphabetically precede the filename`model-settings.json`. Signed-off-by: xinyitan <[email protected]> Co-authored-by: xinyitan <[email protected]>
1 parent 320b051 commit 6a09225

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

model-mesh-mlserver-adapter/server/adaptmodellayout_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,36 @@ var adaptModelLayoutTests = []adaptModelLayoutTestCase{
405405
},
406406
},
407407
},
408+
409+
// model with filename that alphabetically precedes model-settings.json
410+
411+
{
412+
ModelID: "model-filename-precedes-model-settings",
413+
ModelType: "sklearn",
414+
ModelPath: "data",
415+
InputFiles: []string{
416+
"data/model-settings.json",
417+
"data/aaaaa.json",
418+
},
419+
InputConfig: map[string]interface{}{
420+
"name": "model-name",
421+
"implementation": "mlserver_sklearn.SKLearnModel",
422+
"parameters": map[string]interface{}{
423+
"uri": "./aaaaa.json",
424+
},
425+
},
426+
ExpectedFiles: []string{
427+
"model-settings.json",
428+
"aaaaa.json",
429+
},
430+
ExpectedConfig: map[string]interface{}{
431+
"name": "model-filename-precedes-model-settings",
432+
"implementation": "mlserver_sklearn.SKLearnModel",
433+
"parameters": map[string]interface{}{
434+
"uri": filepath.Join(generatedMlserverModelsDir, "model-filename-precedes-model-settings", "aaaaa.json"),
435+
},
436+
},
437+
},
408438
}
409439

410440
func TestAdaptModelLayoutForRuntime(t *testing.T) {

model-mesh-mlserver-adapter/server/server.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,18 @@ func adaptModelLayoutForRuntime(rootModelDir, modelID, modelType, modelPath, sch
190190
// check if the config file exists
191191
// if it does, we assume files are in the "native" repo structure
192192
assumeNativeLayout := false
193-
for _, f := range files {
193+
configFileIndex := -1
194+
for i, f := range files {
194195
if f.Name() == mlserverRepositoryConfigFilename {
195196
assumeNativeLayout = true
197+
configFileIndex = i
196198
break
197199
}
198200
}
201+
// always process the config file first to ensure that uri conversion within config file is correct
202+
if configFileIndex > 0 {
203+
files[0], files[configFileIndex] = files[configFileIndex], files[0]
204+
}
199205
if assumeNativeLayout {
200206
err = adaptNativeModelLayout(files, modelID, modelPath, schemaPath, mlserverModelIDDir, log)
201207
} else {

0 commit comments

Comments
 (0)