python3Packages.chromadb: fixes#412528
Conversation
14d2ff3 to
4d37579
Compare
|
@fabaff Please take a look. |
|
Last of the pre-1.0.0 releases. Required by some legacy AI packages. changelog: https://github.com/chroma-core/chroma/releases/tag/0.6.3
Pins old version to support legacy AI packages.
dotlambda
left a comment
There was a problem hiding this comment.
@GaetanLepage This is unacceptable.
|
|
||
| chromadb = callPackage ../development/python-modules/chromadb { }; | ||
|
|
||
| chromadb_0 = callPackage ../development/python-modules/chromadb/0.nix { }; |
There was a problem hiding this comment.
We don't do versioned attributes in python3Packages! Only for very exceptional cases.
There was a problem hiding this comment.
Sorry. I knew that this should be avoided, but could be done when necessary. It seemed legit in this case.
Is there a reference describing what would be qualified as an "exceptional case"?
There was a problem hiding this comment.
I don't think so. But one necessary condition should be that it's used more than once across nixpkgs.
There was a problem hiding this comment.
But one necessary condition should be that it's used more than once across nixpkgs.
That's fair indeed.
There was a problem hiding this comment.
How does this plan sound:
- I'll move the chromadb 0.6.3 derivation into its one client (
vectorcode) under alet. - I'll revert the chromadb_0 commit
- I'll edit
chromadb1.0.x to put the maturin build hook innativeBuildInputs
Is that acceptable?
There was a problem hiding this comment.
Should be inside packageOverrides inside let.
| with python3Packages; | ||
| [ | ||
| chromadb | ||
| chromadb_0 |
There was a problem hiding this comment.
Sorry, I'll fix it.
| build-system = [ | ||
| setuptools | ||
| setuptools-scm | ||
| rustPlatform.maturinBuildHook |
There was a problem hiding this comment.
This belongs in nativeBuildInputs as it's not a Python package.
| protobuf, | ||
| rustc, | ||
| rustPlatform, | ||
| pkgs, # zstd hidden by python3Packages.zstd |
There was a problem hiding this comment.
Use zstd-c and set zstd-c = pkgs.zstd in python-packages.nix.
On aarch64-linux onnxruntime cannot be imported, as it attempts to read /sys/devices/system/cpu, which does not exist in the sandbox. As onnxruntime cannot be imported, chromadb cannot be tested. However, this issue only applies in the build sandbox, not at runtime. Therefore the package doesn't need to be marked broken. Updated the condition to check `buildPlatform` instead of `hostPlatform`, as the issue relates to the build sandbox and is not a runtime issue. Use `isAarch` and `isLinux` attributes instead of doing string comparison. --- See #412528 (comment) And upstream onnxruntime issue: microsoft/onnxruntime#10038
On aarch64-linux onnxruntime cannot be imported, as it attempts to read /sys/devices/system/cpu, which does not exist in the sandbox. As onnxruntime cannot be imported, chromadb cannot be tested. However, this issue only applies in the build sandbox, not at runtime. Therefore the package doesn't need to be marked broken. Updated the condition to check `buildPlatform` instead of `hostPlatform`, as the issue relates to the build sandbox and is not a runtime issue. Use `isAarch` and `isLinux` attributes instead of doing string comparison. --- See NixOS#412528 (comment) And upstream onnxruntime issue: microsoft/onnxruntime#10038
|
I believe this PR broke the nixos module test for chromadb, it seems to have gone from passing to not passing after it #442520 (comment). |
chromadb: updates and backwards compatibility
with libin meta blockpython3Packages.chromadb_0. Many packages in the AI ecosystem aren't ready for 1.0 and can usechromadb_0.Dependents
python3Packages.langchain-chromato 0.2.4 which needs chromadb >= 1.0.9vectorcodeto usechroma_0Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.