Skip to content

Commit 514f604

Browse files
committed
refactor(backend): extract getAnnotationFilepath helper
The 'prefer modelspec.AnnotationFilepath, fall back to the legacy dragonflyoss key' pattern was duplicated across fetch.go, fetch_by_d7y.go, and pull_by_d7y.go (six call sites total). Centralize it in backend.getAnnotationFilepath so each caller is a one-liner and future changes to the annotation resolution live in one place. Addresses review feedback on PR #468 (gemini). Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
1 parent f155ccb commit 514f604

4 files changed

Lines changed: 43 additions & 54 deletions

File tree

pkg/backend/annotation.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright 2025 The CNAI Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package backend
18+
19+
import (
20+
legacymodelspec "github.com/dragonflyoss/model-spec/specs-go/v1"
21+
modelspec "github.com/modelpack/model-spec/specs-go/v1"
22+
)
23+
24+
// getAnnotationFilepath returns the filepath stored on a descriptor's
25+
// annotations, preferring the modelpack key and falling back to the legacy
26+
// dragonflyoss key so older artifacts remain readable. Returns empty string
27+
// when neither key is present.
28+
func getAnnotationFilepath(annotations map[string]string) string {
29+
if annotations == nil {
30+
return ""
31+
}
32+
if path := annotations[modelspec.AnnotationFilepath]; path != "" {
33+
return path
34+
}
35+
return annotations[legacymodelspec.AnnotationFilepath]
36+
}

pkg/backend/fetch.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ import (
2525
"time"
2626

2727
"github.com/bmatcuk/doublestar/v4"
28-
legacymodelspec "github.com/dragonflyoss/model-spec/specs-go/v1"
29-
modelspec "github.com/modelpack/model-spec/specs-go/v1"
3028
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
3129
"github.com/sirupsen/logrus"
3230
"golang.org/x/sync/errgroup"
@@ -78,10 +76,7 @@ func (b *backend) Fetch(ctx context.Context, target string, cfg *config.Fetch) e
7876
for _, layer := range manifest.Layers {
7977
for _, pattern := range cfg.Patterns {
8078
if anno := layer.Annotations; anno != nil {
81-
path := anno[modelspec.AnnotationFilepath]
82-
if path == "" {
83-
path = anno[legacymodelspec.AnnotationFilepath]
84-
}
79+
path := getAnnotationFilepath(anno)
8580
// Use doublestar.PathMatch for pattern matching to support ** recursive matching
8681
// PathMatch uses the system's native path separator (like filepath.Match) while
8782
// also supporting recursive patterns like **/*.json
@@ -120,14 +115,7 @@ func (b *backend) Fetch(ctx context.Context, target string, cfg *config.Fetch) e
120115
default:
121116
}
122117

123-
var annoFilepath string
124-
if layer.Annotations != nil {
125-
if layer.Annotations[modelspec.AnnotationFilepath] != "" {
126-
annoFilepath = layer.Annotations[modelspec.AnnotationFilepath]
127-
} else {
128-
annoFilepath = layer.Annotations[legacymodelspec.AnnotationFilepath]
129-
}
130-
}
118+
annoFilepath := getAnnotationFilepath(layer.Annotations)
131119

132120
logrus.Debugf("fetch: processing layer %s", layer.Digest)
133121
if err := retrypolicy.Do(ctx, func(rctx context.Context) error {

pkg/backend/fetch_by_d7y.go

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ import (
3131
common "d7y.io/api/v2/pkg/apis/common/v2"
3232
dfdaemon "d7y.io/api/v2/pkg/apis/dfdaemon/v2"
3333
"github.com/bmatcuk/doublestar/v4"
34-
legacymodelspec "github.com/dragonflyoss/model-spec/specs-go/v1"
35-
modelspec "github.com/modelpack/model-spec/specs-go/v1"
3634
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
3735
"github.com/sirupsen/logrus"
3836
"golang.org/x/sync/errgroup"
@@ -81,10 +79,7 @@ func (b *backend) fetchByDragonfly(ctx context.Context, target string, cfg *conf
8179
for _, layer := range manifest.Layers {
8280
for _, pattern := range cfg.Patterns {
8381
if anno := layer.Annotations; anno != nil {
84-
path := anno[modelspec.AnnotationFilepath]
85-
if path == "" {
86-
path = anno[legacymodelspec.AnnotationFilepath]
87-
}
82+
path := getAnnotationFilepath(anno)
8883
// Use doublestar.PathMatch for pattern matching to support ** recursive matching
8984
// PathMatch uses the system's native path separator (like filepath.Match) while
9085
// also supporting recursive patterns like **/*.json
@@ -168,14 +163,7 @@ func (b *backend) fetchByDragonfly(ctx context.Context, target string, cfg *conf
168163

169164
// fetchLayerByDragonfly handles downloading and extracting a single layer via Dragonfly.
170165
func fetchLayerByDragonfly(ctx context.Context, pb *internalpb.ProgressBar, client dfdaemon.DfdaemonDownloadClient, ref Referencer, manifest ocispec.Manifest, desc ocispec.Descriptor, authToken string, cfg *config.Fetch) error {
171-
var annoFilepath string
172-
if desc.Annotations != nil {
173-
if desc.Annotations[modelspec.AnnotationFilepath] != "" {
174-
annoFilepath = desc.Annotations[modelspec.AnnotationFilepath]
175-
} else {
176-
annoFilepath = desc.Annotations[legacymodelspec.AnnotationFilepath]
177-
}
178-
}
166+
annoFilepath := getAnnotationFilepath(desc.Annotations)
179167

180168
err := retrypolicy.Do(ctx, func(rctx context.Context) error {
181169
logrus.Debugf("fetch: processing layer %s", desc.Digest)
@@ -217,14 +205,7 @@ func downloadAndExtractFetchLayer(ctx context.Context, pb *internalpb.ProgressBa
217205
return fmt.Errorf("failed to resolve output dir: %w", err)
218206
}
219207

220-
var annoFilepath string
221-
if desc.Annotations != nil {
222-
if desc.Annotations[modelspec.AnnotationFilepath] != "" {
223-
annoFilepath = desc.Annotations[modelspec.AnnotationFilepath]
224-
} else {
225-
annoFilepath = desc.Annotations[legacymodelspec.AnnotationFilepath]
226-
}
227-
}
208+
annoFilepath := getAnnotationFilepath(desc.Annotations)
228209

229210
if annoFilepath == "" {
230211
return fmt.Errorf("missing annotation filepath")

pkg/backend/pull_by_d7y.go

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ import (
3030

3131
common "d7y.io/api/v2/pkg/apis/common/v2"
3232
dfdaemon "d7y.io/api/v2/pkg/apis/dfdaemon/v2"
33-
legacymodelspec "github.com/dragonflyoss/model-spec/specs-go/v1"
34-
modelspec "github.com/modelpack/model-spec/specs-go/v1"
3533
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
3634
"github.com/sirupsen/logrus"
3735
"golang.org/x/sync/errgroup"
@@ -192,14 +190,7 @@ func buildBlobURL(ref Referencer, plainHTTP bool, digest string) string {
192190

193191
// processLayer handles downloading and extracting a single layer.
194192
func processLayer(ctx context.Context, pb *internalpb.ProgressBar, client dfdaemon.DfdaemonDownloadClient, ref Referencer, manifest ocispec.Manifest, desc ocispec.Descriptor, authToken string, cfg *config.Pull) error {
195-
var annoFilepath string
196-
if desc.Annotations != nil {
197-
if desc.Annotations[modelspec.AnnotationFilepath] != "" {
198-
annoFilepath = desc.Annotations[modelspec.AnnotationFilepath]
199-
} else {
200-
annoFilepath = desc.Annotations[legacymodelspec.AnnotationFilepath]
201-
}
202-
}
193+
annoFilepath := getAnnotationFilepath(desc.Annotations)
203194

204195
err := retrypolicy.Do(ctx, func(rctx context.Context) error {
205196
logrus.Debugf("pull: processing layer %s", desc.Digest)
@@ -236,14 +227,7 @@ func downloadAndExtractLayer(ctx context.Context, pb *internalpb.ProgressBar, cl
236227
return fmt.Errorf("failed to resolve extract dir: %w", err)
237228
}
238229

239-
var annoFilepath string
240-
if desc.Annotations != nil {
241-
if desc.Annotations[modelspec.AnnotationFilepath] != "" {
242-
annoFilepath = desc.Annotations[modelspec.AnnotationFilepath]
243-
} else {
244-
annoFilepath = desc.Annotations[legacymodelspec.AnnotationFilepath]
245-
}
246-
}
230+
annoFilepath := getAnnotationFilepath(desc.Annotations)
247231

248232
if annoFilepath == "" {
249233
return fmt.Errorf("missing annotation filepath")

0 commit comments

Comments
 (0)