feat: add flake-based OpenViking service package#1328
feat: add flake-based OpenViking service package#1328CUexter wants to merge 1 commit intovolcengine:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a Nix flake that builds a Python virtualenv for the OpenViking service by resolving a published openviking wheel (instead of building from the repo checkout), using a small pyproject.toml workspace under nix/openviking-env.
Changes:
- Add
nix/openviking-env/pyproject.tomlto pinopenviking==0.2.10for service consumption. - Add a root
flake.nixthat usesuv2nix+pyproject-nixto produce a virtualenv package from the published wheel.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| nix/openviking-env/pyproject.toml | Defines a minimal Python project used to resolve/pin the published openviking wheel. |
| flake.nix | Introduces a Nix flake that builds a virtualenv from the nix/openviking-env workspace dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| outputs = { nixpkgs, pyproject-nix, uv2nix, pyproject-build-systems, ... }: | ||
| let | ||
| system = "x86_64-linux"; | ||
| pkgs = import nixpkgs { inherit system; }; |
There was a problem hiding this comment.
The flake hard-codes system = "x86_64-linux", which makes evaluation/builds on other systems awkward (e.g., nix build on aarch64-linux/macOS won’t expose a packages.<localSystem> attr). Consider generating outputs via an eachSystem/genAttrs pattern so the flake evaluates cleanly across systems, and only defines the package for supported ones (or errors with a clear message).
| packages.${system} = { | ||
| default = publishedOpenVikingServiceEnv; | ||
| openviking = publishedOpenVikingServiceEnv; | ||
| }; |
There was a problem hiding this comment.
The flake hard-codes system = "x86_64-linux", which makes evaluation/builds on other systems awkward (e.g., nix build on aarch64-linux/macOS won’t expose a packages.<localSystem> attr). Consider generating outputs via an eachSystem/genAttrs pattern so the flake evaluates cleanly across systems, and only defines the package for supported ones (or errors with a clear message).
| "openviking==0.2.10", | ||
| ] | ||
|
|
||
| [tool.uv] |
There was a problem hiding this comment.
If this workspace is intended to be metadata/dependency-resolution only (i.e., not a buildable/installable project itself), it’s worth making that explicit to avoid accidental attempts to build/package openviking-env (which can happen depending on how tooling consumes the workspace). Consider either adding a minimal [build-system] section (so it can be built reliably if needed) or explicitly disabling packaging for the root project in the tool configuration (e.g., via a uv setting) to match the intent stated in the header comment.
| [tool.uv] | |
| [tool.uv] | |
| # This workspace is only used for dependency resolution; do not package it. | |
| package = false |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
Description
Related Issue
Type of Change
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes