-
Notifications
You must be signed in to change notification settings - Fork 646
coglet support #2586
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
michaeldwan
wants to merge
21
commits into
main
Choose a base branch
from
md/cog-wheel-embedding
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
coglet support #2586
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
853190e to
f97e859
Compare
- 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.
Python 3.8 reached end-of-life in October 2024. This change: - Updates minimum Python version from 3.8 to 3.9 - Updates pyproject.toml requires-python - Removes py38 from tox test matrix - Updates all test fixtures and test cases - Updates documentation
The CI test matrix is derived from these classifiers via the build-and-inspect-python-package action.
- Update test_python_37_deprecated to expect 3.9 in error message - Remove obsolete 'fast loader' assertions that were for Python <3.9
Removes Python 3.8 resolution markers and dependencies that were only needed for Python <3.9.
- 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
18dd742 to
8ecdcf2
Compare
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.
nothing to see here, check back later