-
Notifications
You must be signed in to change notification settings - Fork 656
feat(wip): Registry agnostic providers #2668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
markphelps
wants to merge
15
commits into
main
Choose a base branch
from
registry-agnostic-providers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
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
Implements the Provider interface for Replicate's r8.im registry: - MatchesRegistry() checks for r8.im and global registry host - Login() implements the browser-based token flow (extracted from cli/login.go) - LoginWithOptions() provides configurable token-stdin support - PrePush/PostPush are no-ops for now (analytics still in push.go) This extracts the Replicate-specific login logic into the provider abstraction, making it reusable and testable independently.
Adds provider registry functionality: - Registry type with ForImage() and ForHost() methods for provider lookup - ExtractHost() to parse registry host from image names (handles ports, tags, digests) - DefaultRegistry() singleton for global access - setup.Init() to initialize default registry with Replicate and Generic providers Provider order: Replicate (specific) -> Generic (fallback for any OCI registry)
Refactors the login command to use the provider abstraction: - Initializes provider registry with setup.Init() - Looks up provider by registry host - Replicate provider handles its own token-based auth flow - Generic provider returns ErrUseDockerLogin with helpful message The Replicate-specific login logic (token verification, browser flow) now lives in pkg/provider/replicate/replicate.go, making login.go a thin dispatcher that delegates to the appropriate provider. Also exports LoginToRegistry() for programmatic use by other commands.
Updates push command to use the provider abstraction: - Initializes provider registry with setup.Init() - Detects target registry via provider.ForImage() - Uses isReplicate flag for provider-specific behavior - Local image push (--local-image) now explicitly Replicate-only with clear error - 404 errors now show registry-appropriate messages - Replicate model URL only shown when pushing to Replicate This makes the push command work with any OCI registry while maintaining Replicate-specific features and helpful error messages.
Updates command examples and error messages to use generic registry placeholders instead of hardcoded r8.im references: - 'cog push registry.example.com/your-username/model-name' - 'cog pull registry.example.com/your-username/model-name' This makes the documentation more neutral while still being clear about how to use the commands with any OCI registry.
When --x-fast flag is used with a non-Replicate registry, instead of failing or silently ignoring: - Shows a warning that fast push is Replicate-only - Automatically falls back to standard push This maintains backward compatibility while clearly communicating the feature limitation to users.
Analytics (coglog) calls are now only made when pushing to Replicate: - Moved config/image detection before Docker client creation - Determine isReplicate flag before initializing analytics - logClient.StartPush() and EndPush() only called for Replicate - Simplified error handling with endPushWithError helper This ensures non-Replicate push operations don't attempt to contact Replicate's analytics endpoints, improving privacy and performance when using other registries.
- Add documentation for pushing to non-Replicate registries (GHCR, GCR, etc.) - Clarify that 'cog login' is for Replicate; use 'docker login' for others - Add examples showing multi-registry workflows
Major refactoring to move registry-specific logic into provider hooks: Provider interface changes: - Add PushOptions struct to consolidate all push parameters - PrePush(ctx, opts) validates options and starts analytics - PostPush(ctx, opts, pushErr) ends analytics and shows messages GenericProvider: - PrePush() errors on --local-image (not supported) - PrePush() warns on --x-fast (not supported, falls back) - PostPush() shows simple success message ReplicateProvider: - PrePush() starts coglog analytics, shows 'Fast push enabled' - PostPush() ends analytics, shows Replicate URL on success - PostPush() returns Replicate-specific error messages for 404s push.go is now much cleaner: - No more scattered isReplicate checks (reduced from 6+ to 1) - Single PrePush/PostPush call handles all provider-specific behavior - Generic flow: setup -> PrePush -> build -> push -> PostPush The remaining p.Name() check is for disabling FastPush after warning, which could be improved with a SupportsFastPush() method in the future.
GenericProvider now handles login directly by prompting for username and password, then saving credentials via Docker's credential system. Changes: - GenericProvider.Login() prompts interactively instead of returning ErrUseDockerLogin - Remove ErrUseDockerLogin since it's no longer needed - Update login.go to remove ErrUseDockerLogin handling - Update help text to reflect that 'cog login' works for all registries
- Add --registry to Global Options in cli.md and llms.txt - Update cog login examples to show --registry usage for non-Replicate registries - Clarify that --token-stdin is Replicate-specific
Replace string-based '404' error checking with typed NotFoundError:
- Add isRepositoryNotFoundError() to detect NAME_UNKNOWN registry errors
- Convert NAME_UNKNOWN to NotFoundError{Object: "repository"} in Push()
- Update ReplicateProvider.PostPush() to use command.IsNotFoundError()
- Remove string wrapper from StandardPush() since error is now typed
- Add tests for NotFoundError handling in PostPush
Signed-off-by: Mark Phelps <[email protected]>
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.
No description provided.