feat: support pull the blob from dragonfly unix socket#193
Conversation
There was a problem hiding this comment.
Hello @chlins, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello everyone, gemini-code-assist here to provide a summary of this pull request. This PR introduces a new feature to the modctl pull command, allowing users to leverage Dragonfly for pulling model artifacts. Specifically, it adds support for connecting to a Dragonfly daemon via a Unix socket endpoint to download blobs. This enables the use of Dragonfly's P2P capabilities for more efficient distribution of large model files during the pull process, particularly when using the --extract-from-remote flag.
Highlights
- Dragonfly Integration: Adds a new mechanism to the
pullcommand to utilize Dragonfly for downloading model blobs. - New Command Flag: Introduces the
--d7y-endpointflag to specify the Dragonfly gRPC endpoint (expected to be a Unix socket path). - Backend Implementation: Adds a new
PullByDragonflymethod to the backend interface and provides its implementation in a new file (pkg/backend/pull_by_d7y.go). This implementation handles connecting to the Dragonfly daemon, fetching the manifest, obtaining authentication tokens, and initiating download tasks for each layer. - Dependency Updates: Adds necessary dependencies for interacting with Dragonfly API and gRPC (
d7y.io/api/v2,google.golang.org/grpc) and updates some existing dependencies. - Progress Bar Enhancement: Modifies the progress bar to automatically remove completed bars and adds a method to retrieve a specific bar.
- Referencer Update: Adds a
Domain()method to theReferencerinterface and its implementation.
Changelog
Click here to see the changelog
- cmd/pull.go
- Reordered import statements (lines 23-27).
- Added
--d7y-endpointflag to the pull command, bound topullConfig.DragonflyEndpoint(lines 58-58). - Modified
runPullfunction to callb.PullByDragonflyifpullConfig.DragonflyEndpointis set, otherwise calls the existingb.Pull(lines 76-83).
- go.mod
- Added dependency
d7y.io/api/v2 v2.1.39(line 6). - Added dependency
google.golang.org/grpc v1.72.2(line 27). - Added indirect dependency
github.com/envoyproxy/protoc-gen-validate v1.2.1(line 47). - Added indirect dependency
go.opentelemetry.io/auto/sdk v1.1.0(line 90). - Updated indirect dependency
go.opentelemetry.io/otelfrom v1.32.0 to v1.34.0 (line 93). - Updated indirect dependency
go.opentelemetry.io/otel/metricfrom v1.32.0 to v1.34.0 (line 106). - Updated indirect dependency
go.opentelemetry.io/otel/sdkfrom v1.32.0 to v1.34.0 (line 107). - Updated indirect dependency
go.opentelemetry.io/otel/sdk/metricfrom v1.32.0 to v1.34.0 (line 109). - Updated indirect dependency
go.opentelemetry.io/otel/tracefrom v1.32.0 to v1.34.0 (line 110). - Updated indirect dependency
google.golang.org/genproto/googleapis/apifrom v0.0.0-20241209162323-e6fa225c2576 to v0.0.0-20250218202821-56aae31c358a (line 117). - Updated indirect dependency
google.golang.org/genproto/googleapis/rpcfrom v0.0.0-20241223144023-3abc09e42ca8 to v0.0.0-20250519155744-55703ea1f237 (line 118). - Updated indirect dependency
google.golang.org/protobuffrom v1.36.1 to v1.36.6 (line 119).
- Added dependency
- go.sum
- Added checksums for
d7y.io/api/v2 v2.1.39(lines 1-2). - Added checksums for
github.com/envoyproxy/protoc-gen-validate v1.2.1(lines 69-70). - Added checksums for
go.opentelemetry.io/auto/sdk v1.1.0(lines 254-255). - Updated checksums for
go.opentelemetry.io/otel(lines 260-261). - Updated checksums for
go.opentelemetry.io/otel/metric(lines 286-287). - Updated checksums for
go.opentelemetry.io/otel/sdk(lines 288-289). - Updated checksums for
go.opentelemetry.io/otel/sdk/metric(lines 292-293). - Updated checksums for
go.opentelemetry.io/otel/trace(lines 294-295). - Updated checksums for
google.golang.org/genproto/googleapis/api(lines 343-344). - Updated checksums for
google.golang.org/genproto/googleapis/rpc(lines 345-346). - Updated checksums for
google.golang.org/grpc(lines 347-348). - Updated checksums for
google.golang.org/protobuf(lines 349-350).
- Added checksums for
- internal/pb/pb.go
- Added
mpbv8.PopCompletedMode()option toNewProgressBar(line 52). - Modified
Addmethod to return the reader directly if it's nil (lines 107-111). - Added
Getmethod toProgressBarto retrieve a bar by name (lines 115-120).
- Added
- pkg/backend/backend.go
- Added
PullByDragonflymethod to theBackendinterface (lines 43-44).
- Added
- pkg/backend/build.go
- Changed
modelspec.MediaTypeModelDoctomodelspec.MediaTypeModelDocRawinNewDocProcessorcall (line 173).
- Changed
- pkg/backend/pull_by_d7y.go
- Added new file
pull_by_d7y.gocontaining the implementation forPullByDragonflyand helper functions (getAuthToken,buildBlobURL,processLayer,downloadAndExtractLayer,extractTar).
- Added new file
- pkg/backend/referencer.go
- Added
Domain()method to theReferencerinterface (lines 29-30). - Added
Domain()method implementation toreferencerstruct (lines 71-73).
- Added
- pkg/backend/referencer_test.go
- Added
TestReferencer_Domaintest function (lines 73-80).
- Added
- pkg/config/pull.go
- Added
DragonflyEndpointfield to thePullstruct (line 41). - Initialized
DragonflyEndpointto an empty string inNewPull(line 54). - Added validation to
Validatemethod to ensureDragonflyEndpointis only used whenExtractFromRemoteis true (lines 70-73).
- Added
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A socket opens wide,
Dragonfly's power inside.
Blobs fly fast and free,
P2P efficiency.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for pulling blobs using a Dragonfly gRPC service, specifically mentioning hardlinking via a Unix socket. This is a valuable addition for leveraging Dragonfly's capabilities. The implementation adds a new command flag, updates the configuration, introduces a new backend function PullByDragonfly, and includes necessary dependency updates.
The core logic for interacting with the Dragonfly daemon and processing layers concurrently seems well-structured. The addition of the DragonflyEndpoint flag and the validation logic in the pull configuration are appropriate.
Overall, the changes are a good step towards integrating Dragonfly. However, I've identified a couple of areas that could be improved for robustness and clarity, particularly regarding the gRPC connection security and the handling of the remote client type.
Summary of Findings
- Unrelated change in
build.go: A change fromMediaTypeModelDoctoMediaTypeModelDocRawwas included inpkg/backend/build.go. This seems unrelated to the PR's main goal and could potentially be moved to a separate PR for better isolation and review. - Insecure gRPC connection: The gRPC connection to the Dragonfly endpoint uses insecure credentials. While suitable for Unix sockets, this is a security risk if the endpoint can be a network address. The flag description should clarify the intended use case (Unix sockets only) or add TLS configuration options.
- Fragile type assertion: The code relies on a type assertion (
src.Client.(*auth.Client)) which could break if the underlying client type changes in the future. A more robust method for accessing authentication details from the remote client should be considered. - Minor code style/redundancy: A minor check (
if reader != nil) was added ininternal/pb/pb.go.Addwhich is slightly redundant asProxyReaderwould likely handle a nil reader appropriately. (Not commented due to severity settings) - Import reordering: Imports were reordered in
cmd/pull.go. (Not commented due to severity settings)
Merge Readiness
The pull request introduces a valuable feature for integrating with Dragonfly. I've identified a few medium-severity issues related to potential configuration misuse (insecure gRPC for network endpoints) and code fragility (type assertion). I recommend addressing these points before merging to ensure the feature is robust and secure. I am unable to approve this pull request directly; please have other reviewers review and approve this code before merging.
fc6cbd0 to
0c75de6
Compare
e77e921 to
0a0d2f1
Compare
0a0d2f1 to
fbf2722
Compare
Signed-off-by: chlins <chlins.zhang@gmail.com>
fbf2722 to
b2f227f
Compare
No description provided.