-
Notifications
You must be signed in to change notification settings - Fork 652
Add COG_WHEEL env var for runtime selection (cog/coglet/coglet-alpha) #2616
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
Conversation
- Create pkg/dockerfile/wheel.go with WheelSource enum and WheelConfig - Add ParseCogWheel() to parse COG_WHEEL env var values: cog, coglet, coglet-alpha, URLs, or file paths - Add GetWheelConfig() to determine wheel based on COG_WHEEL and cog_runtime flag - Create pkg/wheels/ package with simple go:embed for cog.whl and coglet.whl - Add script/generate-wheels to build both wheels to dist/ and copy to pkg/wheels/ - Add script/build-coglet-server for linux/amd64 coglet Go binary - Simplify Makefile: remove complex wheel targets, use go generate - Update StandardGenerator and migrator to use wheels.ReadCogWheel() Closes cog-mrj, cog-a86, cog-i3w
Refactor installCog() to dispatch based on GetWheelConfig(): - installEmbeddedCogWheel() for COG_WHEEL=cog (default when cog_runtime: false) - installEmbeddedCogletWheel() for COG_WHEEL=coglet - installCogletAlpha() for COG_WHEEL=coglet-alpha (default when cog_runtime: true) - installWheelFromURL() for COG_WHEEL=https://... - installWheelFromFile() for COG_WHEEL=/path/to/file.whl Backwards compatible: existing behavior unchanged without COG_WHEEL set. Closes cog-1xb
Test coverage for all COG_WHEEL values: - Default with cog_runtime: false -> embedded cog wheel - Default with cog_runtime: true -> coglet-alpha (PinnedCogletURL) - COG_WHEEL=cog -> embedded cog wheel (overrides cog_runtime) - COG_WHEEL=coglet -> embedded coglet wheel - COG_WHEEL=coglet-alpha -> PinnedCogletURL - COG_WHEEL=https://... -> URL install (with/without coglet env vars) - COG_WHEEL=/path/to/file.whl -> local file install Closes cog-6q8
Better separation of concerns - wheel selection logic belongs with wheel embedding, not Dockerfile generation.
- Update generate-wheels to use COG_WHEEL/COGLET_WHEEL env vars if set - Add astral-sh/setup-uv@v7 to test-go job for coglet wheel building
- Remove go:generate directives from pkg/config/compatibility.go - Add script/generate-compat for manual matrix regeneration - Document in CONTRIBUTING.md when/how to update compatibility matrices This speeds up 'go generate ./...' by only running wheel generation, not the slow compatgen tool that rarely needs to run.
- Update package-data pattern in coglet/pyproject.toml to match bin/coglet-server-* instead of cog-* - Fix gocritic appendAssign lint error in installWheelFromURL
- Add script/build-wheels to build both cog and coglet wheels to dist/ - Replace go:generate script call with simple cp commands - Delete script/generate-wheels (no longer needed) - Add 'make wheel' target - Simplify CI: use merge-multiple to download all artifacts at once - Remove COG_WHEEL/COGLET_WHEEL env var setup from CI jobs
pip requires wheel filenames to follow PEP 427 format (e.g., cog-0.16.10-py3-none-any.whl). Previously we renamed wheels to simple names (cog.whl) which pip rejected. Switch from []byte embedding to embed.FS which preserves the original versioned filenames. Panic instead of returning error since missing wheels indicate a broken build.
- Fix binary path in go_cog.py to use cog/bin/coglet-server-{os}-{arch}
- Add 'pip uninstall cog' before coglet install to avoid conflicts with
pre-installed cog in base images (e.g. r8.im/cog-base)
- Add wheel count assertion to prevent stale wheels from being embedded
- Clean old wheels in build-wheels script before building new ones
markphelps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!!
There was a problem hiding this 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 PR enables runtime selection between cog, coglet, and coglet-alpha via the COG_WHEEL environment variable, providing flexibility in choosing which Python runtime wheel to use during container builds.
- Introduces a new
pkg/wheelspackage for centralized wheel management and selection logic - Embeds both cog and coglet wheels into the cog binary for offline installation
- Updates CI workflows to run integration tests against all three runtime variants (cog, coglet, coglet-alpha)
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pkg/wheels/wheels.go |
Core wheel selection logic with support for embedded wheels, URLs, and local files |
pkg/wheels/wheels_test.go |
Comprehensive test coverage for wheel parsing and configuration |
pkg/dockerfile/standard_generator.go |
Refactored wheel installation to support multiple sources based on COG_WHEEL env var |
pkg/dockerfile/standard_generator_test.go |
Tests for all wheel selection scenarios and runtime combinations |
pkg/migrate/migrator_v1_v1fast.go |
Updated to use new wheels package API |
pkg/dockerfile/cog_embed.go |
Removed - functionality moved to pkg/wheels |
pkg/dockerfile/cog_embed_test.go |
Removed - tests moved to pkg/wheels |
pkg/config/compatibility.go |
Removed inline go:generate directives in favor of centralized script |
script/generate-compat |
New script for regenerating compatibility matrices |
script/build-wheels |
New script for building both cog and coglet wheels |
script/build-coglet-server |
New script for building coglet-server binary |
coglet/python/cog/command/go_cog.py |
Updated binary path from root to bin/ subdirectory |
coglet/pyproject.toml |
Updated package data to include binaries from new location |
Makefile |
Simplified build process to use generate target and new build scripts |
.github/workflows/ci.yaml |
Added matrix strategy to test all three runtimes with appropriate failure handling |
.goreleaser.yaml |
Updated to use go generate for wheel preparation |
.gitignore |
Updated paths for new wheel locations and coglet binaries |
CONTRIBUTING.md |
Added documentation for compatibility matrix regeneration |
tox.ini |
Added COG_WHEEL to pass-through environment variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Enables runtime selection between cog and coglet via the
COG_WHEELenvironment variable.Supported values:
cog- Embedded cog wheel (default whencog_runtime: false)coglet- Embedded coglet wheel (new, from this repo'scoglet/directory)coglet-alpha- PinnedCogletURL (default whencog_runtime: true)https://...- Custom wheel URL/path/to/file.whl- Local wheel fileChanges:
COG_WHEELenv var parsing inpkg/wheels/wheels.goStandardGeneratorcog,coglet,coglet-alpha)CI Configuration:
cog: Full test suite (required to pass)coglet-alpha: Passes, stop after 5 failurescoglet: Exit on first failure (needs follow-up work for error message alignment)Split from #2586 - Python 3.8 removal moved to separate branch (
md/drop-python38-support)