Skip to content

feat: support attach command to attach a file to existed model artifact#141

Merged
chlins merged 1 commit intomainfrom
feat/attach
Mar 31, 2025
Merged

feat: support attach command to attach a file to existed model artifact#141
chlins merged 1 commit intomainfrom
feat/attach

Conversation

@chlins
Copy link
Member

@chlins chlins commented Mar 28, 2025

This pull request introduces a new attach command to the modctl tool and enhances the existing build and push commands with a spinner for better user feedback during the nydusification process. Additionally, it includes necessary updates to the go.mod file and adds a new backend function to support the attach command.

New attach command:

  • Added attach.go to implement the attach command, which allows attaching user materials into the model artifact. The command includes various flags for configuration and supports nydusification.
  • Registered the attachCmd in root.go to make the command available in the CLI.

Enhancements to existing commands:

  • Added a spinner to the build command to provide visual feedback during the nydusification process. [1] [2]
  • Added a spinner to the push command to provide visual feedback during the nydusification process. [1] [2]

Dependency updates:

  • Updated go.mod to include new dependencies for the spinner and color libraries. [1] [2] [3]

Backend support for attach command:

  • Added attach.go in the pkg/backend directory to implement the backend logic for the attach command, including functions for processing layers, building configurations, and sorting layers.

@chlins chlins requested a review from Copilot March 28, 2025 02:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds a new attach command to the modctl tool while enhancing the build and push commands with spinners during the nydusification process, and it updates dependency management accordingly. Key changes include introducing attach.go in both config and backend, refactoring builder functions to use an explicit model config, and integrating spinner feedback into build/push/attach commands.

Reviewed Changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/modelfile/constants*.go Renamed variables and function to export constants and utility functions appropriately.
pkg/config/attach.go Introduced a new configuration type to support the attach command.
pkg/backend/{attach.go,attach_test.go} Added the attach command implementation and tests, including layer replacement logic.
pkg/backend/build/* Refactored builder functions to work with an explicit model config rather than a modelfile.
pkg/backend/push.go, pull.go, fetch.go, remote/client.go Refactored remote client creation to use a common helper with spinner and error improvements.
cmd/{build.go,push.go,attach.go,root.go} Integrated the attach command into the CLI and enhanced user feedback through spinners.
Files not reviewed (1)
  • go.mod: Language not supported
Comments suppressed due to low confidence (2)

pkg/backend/attach.go:165

  • The error message refers to an 'image manifest' while the build context is for a model artifact; consider changing it to 'model manifest'.
return fmt.Errorf("failed to build image manifest: %w", err)

cmd/attach.go:70

  • The field 'StoargeDir' appears to be misspelled; it should likely be 'StorageDir'.
b, err := backend.New(rootConfig.StoargeDir)

Signed-off-by: chlins <chlins.zhang@gmail.com>
Copy link
Member

@gaius-qi gaius-qi left a comment

Choose a reason for hiding this comment

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

LGTM

@chlins chlins merged commit 0a84a63 into main Mar 31, 2025
5 checks passed
@chlins chlins deleted the feat/attach branch March 31, 2025 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants