Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func init() {
// TODO: set the raw flag to true by default in future.
flags.BoolVar(&buildConfig.Raw, "raw", false, "turning on this flag will build model artifact layers in raw format")
flags.BoolVar(&buildConfig.Reasoning, "reasoning", false, "turning on this flag will mark this model as reasoning model in the config")
flags.BoolVar(&buildConfig.NoCreationTime, "no-creation-time", false, "turning on this flag will not set createdAt in the config, which will be helpful for repeated builds")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The help text for this new flag is a bit verbose. For better CLI usability, consider making it more concise. A more idiomatic help message could be something like: Omit creation timestamp for reproducible builds.

Suggested change
flags.BoolVar(&buildConfig.NoCreationTime, "no-creation-time", false, "turning on this flag will not set createdAt in the config, which will be helpful for repeated builds")
flags.BoolVar(&buildConfig.NoCreationTime, "no-creation-time", false, "Omit creation timestamp for reproducible builds")


if err := viper.BindPFlags(flags); err != nil {
panic(fmt.Errorf("bind cache list flags to viper: %w", err))
Expand Down
1 change: 1 addition & 0 deletions pkg/backend/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func (b *backend) Build(ctx context.Context, modelfilePath, workDir, target stri
SourceURL: sourceInfo.URL,
SourceRevision: revision,
Reasoning: cfg.Reasoning,
NoCreationTime: cfg.NoCreationTime,
}, layers)
if err != nil {
return fmt.Errorf("failed to build model config: %w", err)
Expand Down
7 changes: 5 additions & 2 deletions pkg/backend/build/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,18 @@ func BuildModelConfig(modelConfig *buildconfig.Model, layers []ocispec.Descripto
}
}

createdAt := time.Now()
descriptor := modelspec.ModelDescriptor{
CreatedAt: &createdAt,
Family: modelConfig.Family,
Name: modelConfig.Name,
SourceURL: modelConfig.SourceURL,
Revision: modelConfig.SourceRevision,
}

if !modelConfig.NoCreationTime {
createdAt := time.Now()
descriptor.CreatedAt = &createdAt
}
Comment on lines +267 to +270

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This conditional logic is the core of the new feature. To ensure its correctness and prevent future regressions, it's important to add unit tests. Please consider updating TestBuildModelConfig in pkg/backend/build/builder_test.go to cover both scenarios: when NoCreationTime is true (asserting CreatedAt is nil) and when it's false (asserting CreatedAt is set).


diffIDs := make([]godigest.Digest, 0, len(layers))
for _, layer := range layers {
diffIDs = append(diffIDs, layer.Digest)
Expand Down
1 change: 1 addition & 0 deletions pkg/backend/build/config/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ type Model struct {
SourceURL string
SourceRevision string
Reasoning bool
NoCreationTime bool

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve code clarity and maintainability, it's good practice to add doc comments for exported fields. Please add a comment for NoCreationTime explaining its purpose.

	// NoCreationTime omits the creation timestamp for reproducible builds.
	NoCreationTime bool

}
2 changes: 2 additions & 0 deletions pkg/config/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type Build struct {
SourceRevision string
Raw bool
Reasoning bool
NoCreationTime bool
}

func NewBuild() *Build {
Expand All @@ -50,6 +51,7 @@ func NewBuild() *Build {
SourceRevision: "",
Raw: false,
Reasoning: false,
NoCreationTime: false,
}
}

Expand Down