Avoid decoding unrelated profiles
This commit is contained in:
@@ -42,12 +42,14 @@ duplicate detection, and source containment:
|
|||||||
`internal/profile` loads and validates execution profiles from an
|
`internal/profile` loads and validates execution profiles from an
|
||||||
operating-system filesystem or an `fs.FS`. A file contains exactly one YAML
|
operating-system filesystem or an `fs.FS`. A file contains exactly one YAML
|
||||||
document and its trimmed YAML `id` is its only selection identity; filenames do
|
document and its trimmed YAML `id` is its only selection identity; filenames do
|
||||||
not confer authority. Strict selected decoding recognizes the optional
|
not confer authority. Each point lookup reads discovered files once for their
|
||||||
`backend` field, trims its value, and requires a model plus at least one
|
metadata and reuses the selected file's bytes for strict decoding; unrelated
|
||||||
non-blank backend or endpoint. File-backed `extra_params` values are validated
|
profiles are not fully decoded. Strict selected decoding recognizes the
|
||||||
and defensively copied through the shared bounded JSON-value owner before a
|
optional `backend` field, trims its value, and requires a model plus at least
|
||||||
profile is published. OpenAI-compatible reserved-field policy remains with the
|
one non-blank backend or endpoint. File-backed `extra_params` values are
|
||||||
model-client and backend-registry owners.
|
validated and defensively copied through the shared bounded JSON-value owner
|
||||||
|
before a profile is published. OpenAI-compatible reserved-field policy remains
|
||||||
|
with the model-client and backend-registry owners.
|
||||||
|
|
||||||
The overlay repository consults the next repository only when the
|
The overlay repository consults the next repository only when the
|
||||||
higher-precedence repository reports that a profile is absent. A reliably
|
higher-precedence repository reports that a profile is absent. A reliably
|
||||||
|
|||||||
@@ -114,14 +114,14 @@ func loadProfile(ctx context.Context, fsys fs.FS, root string, id string) (*doma
|
|||||||
}
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
if !idMatch {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
prof, err := decodeProfile(data)
|
prof, err := decodeProfile(data)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if idMatch {
|
|
||||||
return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, err)
|
return nil, fmt.Errorf("%w: %s: %v", ErrInvalidYAML, relPath, err)
|
||||||
}
|
}
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
prof.ID = strings.TrimSpace(prof.ID)
|
prof.ID = strings.TrimSpace(prof.ID)
|
||||||
if prof.ID != id {
|
if prof.ID != id {
|
||||||
|
|||||||
58
internal/profile/repository_benchmark_test.go
Normal file
58
internal/profile/repository_benchmark_test.go
Normal file
@@ -0,0 +1,58 @@
|
|||||||
|
package profile
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"testing"
|
||||||
|
"testing/fstest"
|
||||||
|
)
|
||||||
|
|
||||||
|
func BenchmarkProfileRepositoryLookup(b *testing.B) {
|
||||||
|
for _, size := range []int{10, 1000} {
|
||||||
|
b.Run(fmt.Sprintf("catalog-%d", size), func(b *testing.B) {
|
||||||
|
files := fstest.MapFS{
|
||||||
|
"target.yaml": profileMapFile(`
|
||||||
|
id: target
|
||||||
|
endpoint: http://localhost:8000/v1
|
||||||
|
model: target-model
|
||||||
|
extra_params:
|
||||||
|
selected: true
|
||||||
|
`),
|
||||||
|
}
|
||||||
|
metadataNames := []string{"target.yaml"}
|
||||||
|
for i := 1; i < size; i++ {
|
||||||
|
name := fmt.Sprintf("profile-%04d.yaml", i)
|
||||||
|
files[name] = profileMapFile(fmt.Sprintf(`
|
||||||
|
id: profile-%04d
|
||||||
|
endpoint: http://localhost:8000/v1
|
||||||
|
model: unrelated-model
|
||||||
|
temperature: 0.5
|
||||||
|
max_tokens: 500
|
||||||
|
extra_params:
|
||||||
|
provider:
|
||||||
|
order:
|
||||||
|
- first
|
||||||
|
- second
|
||||||
|
`, i))
|
||||||
|
metadataNames = append(metadataNames, name)
|
||||||
|
}
|
||||||
|
|
||||||
|
fsys := &recordingProfileFS{FS: files}
|
||||||
|
repo := NewFSRepository(fsys, ".")
|
||||||
|
b.ReportAllocs()
|
||||||
|
b.ResetTimer()
|
||||||
|
for i := 0; i < b.N; i++ {
|
||||||
|
if _, err := repo.GetProfile(context.Background(), "target"); err != nil {
|
||||||
|
b.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
b.StopTimer()
|
||||||
|
|
||||||
|
for _, name := range metadataNames {
|
||||||
|
if got := fsys.openCount(name); got != b.N {
|
||||||
|
b.Fatalf("metadata %q opens = %d, want %d", name, got, b.N)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -4,9 +4,11 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
"testing/fstest"
|
"testing/fstest"
|
||||||
|
|
||||||
@@ -720,6 +722,70 @@ unknown: true
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestProfileRepositoryReadsSourcesFreshOnEveryLookup(t *testing.T) {
|
||||||
|
newSource := func() (*recordingProfileFS, Repository) {
|
||||||
|
fsys := &recordingProfileFS{FS: fstest.MapFS{
|
||||||
|
"target.yaml": profileMapFile(`
|
||||||
|
id: target
|
||||||
|
endpoint: http://localhost:8000/v1
|
||||||
|
model: target-model
|
||||||
|
`),
|
||||||
|
"unrelated.yaml": profileMapFile(`
|
||||||
|
id: unrelated
|
||||||
|
endpoint: http://localhost:8000/v1
|
||||||
|
model: unrelated-model
|
||||||
|
`),
|
||||||
|
}}
|
||||||
|
return fsys, NewFSRepository(fsys, ".")
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Run("selected source", func(t *testing.T) {
|
||||||
|
fsys, repo := newSource()
|
||||||
|
for lookup := 1; lookup <= 2; lookup++ {
|
||||||
|
got, err := repo.GetProfile(context.Background(), "target")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("lookup %d: %v", lookup, err)
|
||||||
|
}
|
||||||
|
if got.Model != "target-model" {
|
||||||
|
t.Fatalf("lookup %d model = %q", lookup, got.Model)
|
||||||
|
}
|
||||||
|
for _, name := range []string{"target.yaml", "unrelated.yaml"} {
|
||||||
|
if count := fsys.openCount(name); count != lookup {
|
||||||
|
t.Fatalf("%s opens after lookup %d = %d, want %d", name, lookup, count, lookup)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("overlay fallthrough", func(t *testing.T) {
|
||||||
|
primaryFS := &recordingProfileFS{FS: fstest.MapFS{
|
||||||
|
"unrelated.yaml": profileMapFile(`
|
||||||
|
id: unrelated
|
||||||
|
endpoint: http://localhost:8000/v1
|
||||||
|
model: unrelated-model
|
||||||
|
`),
|
||||||
|
}}
|
||||||
|
fallbackFS, fallback := newSource()
|
||||||
|
repo := NewOverlayRepository(NewFSRepository(primaryFS, "."), fallback)
|
||||||
|
|
||||||
|
for lookup := 1; lookup <= 2; lookup++ {
|
||||||
|
got, err := repo.GetProfile(context.Background(), "target")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("lookup %d: %v", lookup, err)
|
||||||
|
}
|
||||||
|
if got.Model != "target-model" {
|
||||||
|
t.Fatalf("lookup %d model = %q", lookup, got.Model)
|
||||||
|
}
|
||||||
|
if count := primaryFS.openCount("unrelated.yaml"); count != lookup {
|
||||||
|
t.Fatalf("primary opens after lookup %d = %d, want %d", lookup, count, lookup)
|
||||||
|
}
|
||||||
|
if count := fallbackFS.openCount("target.yaml"); count != lookup {
|
||||||
|
t.Fatalf("fallback opens after lookup %d = %d, want %d", lookup, count, lookup)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func TestProfileRepositoriesRejectInvalidExecutionSettings(t *testing.T) {
|
func TestProfileRepositoriesRejectInvalidExecutionSettings(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
@@ -849,6 +915,31 @@ type profileRepositorySource struct {
|
|||||||
newRepository func(t *testing.T, files map[string]string) Repository
|
newRepository func(t *testing.T, files map[string]string) Repository
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type recordingProfileFS struct {
|
||||||
|
fs.FS
|
||||||
|
mu sync.Mutex
|
||||||
|
opened []string
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *recordingProfileFS) Open(name string) (fs.File, error) {
|
||||||
|
f.mu.Lock()
|
||||||
|
f.opened = append(f.opened, name)
|
||||||
|
f.mu.Unlock()
|
||||||
|
return f.FS.Open(name)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *recordingProfileFS) openCount(name string) int {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
count := 0
|
||||||
|
for _, opened := range f.opened {
|
||||||
|
if opened == name {
|
||||||
|
count++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return count
|
||||||
|
}
|
||||||
|
|
||||||
func profileRepositorySources() []profileRepositorySource {
|
func profileRepositorySources() []profileRepositorySource {
|
||||||
return []profileRepositorySource{
|
return []profileRepositorySource{
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user