Add file snapshot functionality #3
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for downloading repository snapshots locally with caching capabilities. The implementation includes metadata tracking to avoid re-downloading unchanged files and support for glob pattern filtering.
- Introduces
downloadSnapshotmethod to download complete repository snapshots with progress tracking - Adds
LocalDownloadFileMetadatastructure to track download metadata (commit hash, etag, timestamp) - Implements metadata persistence and validation helpers using file-based storage
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Sources/HuggingFace/Hub/HubClient+Files.swift | Adds snapshot download functionality, metadata helpers, and file hashing utilities; marks several unused parameters with underscore prefix |
| Sources/HuggingFace/Hub/File.swift | Defines LocalDownloadFileMetadata struct and removes self. prefix in FileBatch.init() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,3 +1,4 @@ | |||
| import CryptoKit | |||
There was a problem hiding this comment.
The CryptoKit import is unused in this file. The LocalDownloadFileMetadata struct only stores metadata as strings and does not perform any cryptographic operations. Consider removing this import since SHA256 is only used in HubClient+Files.swift where it's already imported.
| import CryptoKit |
| .filter { filename in | ||
| guard !globs.isEmpty else { return true } | ||
| return globs.contains { glob in | ||
| fnmatch(glob, filename, 0) == 0 |
There was a problem hiding this comment.
The fnmatch function is a C library function that may not be available on all platforms (requires Darwin on Apple platforms or Glibc on Linux). Consider adding appropriate platform-specific imports (#if canImport(Darwin) import Darwin #elseif canImport(Glibc) import Glibc #endif) at the top of the file to ensure cross-platform compatibility, or document platform requirements if this is intentionally platform-specific.
| } | ||
|
|
||
| if Task.isCancelled { | ||
| return repoDestination |
There was a problem hiding this comment.
When a task is cancelled, returning normally may lead to incomplete downloads being treated as successful. Consider throwing a CancellationError instead of returning the destination URL to properly signal the cancellation state: throw CancellationError().
| return repoDestination | |
| throw CancellationError() |
| let filename = metadataPath.lastPathComponent.replacingOccurrences( | ||
| of: ".metadata", | ||
| with: "" | ||
| ) |
There was a problem hiding this comment.
Using replacingOccurrences(of:with:) for extracting the filename is fragile. If a file legitimately contains '.metadata' in its name (e.g., 'model.metadata.json'), this will incorrectly strip that portion. Use deletingPathExtension() if '.metadata' is truly an extension, or String.removingSuffix() to only remove the suffix if it exists at the end.
Follow-up to #2