Conversation
Signed-off-by: chlins <chlins.zhang@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Raw flag to the attach command, enabling the use of “raw” media types when attaching model artifact layers, and makes related updates across the codebase.
- Add
Rawboolean toAttachconfig and wire it through the CLI - Extend
getProcessorto pick raw media types and replace manual slice removal withslices.Delete - Update tests and bump progress bar refresh rate for smoother updates
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/config/attach.go | Added Raw field to Attach struct and initialized it in NewAttach |
| cmd/attach.go | Exposed --raw flag in the attach command |
| pkg/backend/attach.go | Updated getProcessor signature to accept Raw, select raw media types, and use slices.Delete |
| pkg/backend/attach_test.go | Updated test call to getProcessor to include *config.Attach |
| internal/pb/pb.go | Increased progress bar refresh rate from 200ms to 300ms |
Comments suppressed due to low confidence (1)
pkg/backend/attach_test.go:76
- Tests currently only cover the default (
Raw=false) path. Add a test case where&config.Attach{Raw: true}is passed to verify that the processor uses the raw media types.
proc := b.getProcessor(tt.filepath, &config.Attach{})
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new feature to support attaching model artifact layers in raw format and includes several related updates across the codebase. Key changes include adding a
Rawflag to the configuration, modifying the processor logic to handle the raw format, and updating tests and dependencies accordingly.New Feature: Support for Raw Format
Rawflag to theAttachstruct inpkg/config/attach.goand initialized it in theNewAttachfunction. This flag enables attaching model artifact layers in raw format. [1] [2]cmd/attach.gofile to include theRawflag in the CLI options.Processor Logic Enhancements
getProcessorfunction inpkg/backend/attach.goto use theRawflag to determine the appropriate media type for different file types. Added support for raw formats in model config, model weight, model code, and model doc processors.Attachmethod to pass thecfgparameter togetProcessorand replaced manual slice manipulation with theslices.Deletefunction for improved readability and safety.Dependency and Test Updates
slicespackage to the imports inpkg/backend/attach.goto utilize theslices.Deletefunction.pkg/backend/attach_test.goto pass thecfgparameter to thegetProcessorfunction.Minor Adjustments
internal/pb/pb.gofrom 200ms to 300ms for smoother updates.