From bdd4c518accc761e5aeb7bf967016ff17750949e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20de=20Kok?= Date: Wed, 2 Sep 2026 13:03:24 +0000 Subject: [PATCH] kernels: make kernels version check a validator This check the kernel version already when just the metadata is loaded (fail early). Also make it raise an exception and not a warning. If the kernel uses a feature that is not supported by the kernels version, it is not going to work, so we should just raise. Avoids downloading the whole kernel and then having it fail with a hard to understand exception. --- docs/source/kernel-requirements.md | 5 +- kernels/src/kernels/deps.py | 59 ++++++++++++++- kernels/src/kernels/importer.py | 54 +------------ kernels/src/kernels/layer/func.py | 4 +- kernels/src/kernels/layer/layer.py | 4 +- kernels/src/kernels/load.py | 16 ++-- kernels/tests/test_minver.py | 75 ++++++------------- .../get-kernel-check/get-kernel-check-hook.py | 4 +- 8 files changed, 102 insertions(+), 119 deletions(-) diff --git a/docs/source/kernel-requirements.md b/docs/source/kernel-requirements.md index 4c720022..b9a7738a 100644 --- a/docs/source/kernel-requirements.md +++ b/docs/source/kernel-requirements.md @@ -72,8 +72,9 @@ metadata. Currently the following top-level keys are supported: - `kernels-minver` (`str`, optional): the minimum version of the `kernels` Python library required to load the kernel (e.g. `"0.17.0"`). This key is determined by the kernel builder from the features that the kernel uses, - it is not set by kernel authors. The `kernels` library warns when it loads - a kernel that requires a newer version than the one that is installed. + it is not set by kernel authors. The `kernels` library raises an error when + loading a kernel that requires a newer version than the one that is + installed. - `license` (`str`, required): the kernel license in. Refer to the list of [supported license identifiers](https://huggingface.co/docs/hub/repositories-licenses). - `upstream` (`str`, optional): Git-compatible URL (passable to `git clone`) diff --git a/kernels/src/kernels/deps.py b/kernels/src/kernels/deps.py index 17e0ada3..9486c3ed 100644 --- a/kernels/src/kernels/deps.py +++ b/kernels/src/kernels/deps.py @@ -4,7 +4,8 @@ from typing import Generic, Protocol, TypeVar from huggingface_hub.hf_api import HfApi -from kernels_data import KernelDependency, KernelVersion +from kernels_data import KernelDependency, KernelVersion, Version +from packaging.version import InvalidVersion, parse from kernels.archs import _check_arch_incompatibility from kernels.backends import _backend @@ -106,6 +107,57 @@ def validate(self, *, tree: DepTreeNode[LocalKernel | RemoteKernel]) -> None: _check_arch_incompatibility(tree.location.metadata, tree.location.variant_str) +def _installed_version() -> Version | None: + """The installed `kernels` version as a numeric version. + + Pre-release and development suffixes are stripped, since kernel metadata + only records release versions. Without stripping, a development version + like `0.17.0.dev0` would compare as older than the `0.17.0` it implements. + + Returns `None` if the installed version is not a PEP 440 version, which + can happen for versions derived from a VCS checkout. Such a version cannot + be compared, and this check must never make a kernel fail to load. + """ + # Avoid an import cycle. + from kernels import __version__ + + try: + release = parse(__version__).release + except InvalidVersion: + return None + + # packaging < 22 returns a LegacyVersion with `release = None` for + # non-PEP 440 versions instead of raising `InvalidVersion`. + if release is None: + return None + + return Version.from_str(".".join(str(part) for part in release)) + + +class MinverValidator: + """Validate that the installed `kernels` library meets the minimum version + required by a kernel.""" + + def validate(self, *, tree: DepTreeNode[LocalKernel | RemoteKernel]) -> None: + metadata = tree.location.metadata + minver = metadata.kernels_minver + if minver is None: + return + + installed = _installed_version() + if installed is not None and installed < minver: + # Report the verbatim installed version, not the normalized one used + # for comparison, so the message matches what `pip show` reports. + # Avoid an import cycle. + from kernels import __version__ + + raise RuntimeError( + f"Kernel '{metadata.name}' variant '{tree.location.variant_str}' requires " + f"kernels>={minver}, but version {__version__} is installed. " + "Upgrade with: pip install --upgrade kernels" + ) + + @dataclass class AllValidator: """Apply multiple validators to a kernel dependency tree.""" @@ -117,6 +169,11 @@ def validate(self, *, tree: DepTreeNode[LocalKernel | RemoteKernel]) -> None: validator.validate(tree=tree) +def default_validators() -> list[Validator]: + """The validators that are applied to every kernel dependency tree.""" + return [DependencyValidator(), MinverValidator()] + + LoadCache = dict[KernelDependency, DepTreeNode] diff --git a/kernels/src/kernels/importer.py b/kernels/src/kernels/importer.py index eb46f3a8..f53c13ea 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -5,8 +5,7 @@ from pathlib import Path from types import ModuleType -from kernels_data import Metadata, Version -from packaging.version import InvalidVersion, parse +from kernels_data import Metadata from kernels.hf_hub import RepoInfo @@ -94,56 +93,6 @@ def _warn_if_dirty(metadata: Metadata, variant_str: str) -> None: ) -def _installed_version() -> Version | None: - """The installed `kernels` version as a numeric version. - - Pre-release and development suffixes are stripped, since kernel metadata - only records release versions. Without stripping, a development version - like `0.17.0.dev0` would compare as older than the `0.17.0` it implements. - - Returns `None` if the installed version is not a PEP 440 version, which - can happen for versions derived from a VCS checkout. Such a version cannot - be compared, and this check must never make a kernel fail to load. - """ - # Avoid an import cycle. - from kernels import __version__ - - try: - release = parse(__version__).release - except InvalidVersion: - return None - - # packaging < 22 returns a LegacyVersion with `release = None` for - # non-PEP 440 versions instead of raising `InvalidVersion`. - if release is None: - return None - - return Version.from_str(".".join(str(part) for part in release)) - - -def _warn_if_below_minver(metadata: Metadata, variant_str: str) -> None: - """Warn when the installed `kernels` library is older than the minimum - version required by a kernel. - """ - minver = metadata.kernels_minver - if minver is None: - return - - installed = _installed_version() - if installed is not None and installed < minver: - # Report the verbatim installed version, not the normalized one used - # for comparison, so the message matches what `pip show` reports. - from kernels import __version__ - - warnings.warn( - f"Kernel '{metadata.name}' variant '{variant_str}' requires " - f"kernels>={minver}, but version {__version__} is installed. " - "The kernel may not load or work correctly; upgrade with: " - "pip install --upgrade kernels", - stacklevel=3, - ) - - def _import_from_path( variant_path: Path, deps: dict[str, ModuleType], @@ -154,7 +103,6 @@ def _import_from_path( metadata = Metadata.read_from_file(variant_path / "metadata.json") _warn_if_dirty(metadata, variant_path.name) - _warn_if_below_minver(metadata, variant_path.name) module_name = metadata.name.python_name file_path = variant_path / "__init__.py" diff --git a/kernels/src/kernels/layer/func.py b/kernels/src/kernels/layer/func.py index 881258b3..dfd47c72 100644 --- a/kernels/src/kernels/layer/func.py +++ b/kernels/src/kernels/layer/func.py @@ -10,7 +10,7 @@ from kernels.resolver import LockedHubCacheResolver, LockedHubResolver from .._versions import select_revision_or_version -from ..deps import DependencyValidator +from ..deps import AllValidator, default_validators from ..hf_hub import _get_hf_api from ..load import ( get_kernel, @@ -332,7 +332,7 @@ def load(self) -> Type["nn.Module"]: backend=None, kernel=self.kernel_dep, resolver=resolver, - validator=DependencyValidator(), + validator=AllValidator(validators=default_validators()), ) return _get_kernel_func(self, kernel) diff --git a/kernels/src/kernels/layer/layer.py b/kernels/src/kernels/layer/layer.py index fa34b8c8..86d13e8f 100644 --- a/kernels/src/kernels/layer/layer.py +++ b/kernels/src/kernels/layer/layer.py @@ -15,7 +15,7 @@ from kernels.resolver import LockedHubCacheResolver, LockedHubResolver from .._versions import select_revision_or_version -from ..deps import DependencyValidator +from ..deps import AllValidator, default_validators from ..hf_hub import _get_hf_api from ..load import ( get_kernel, @@ -241,7 +241,7 @@ def load(self) -> Type["nn.Module"]: backend=None, kernel=self.kernel_dep, resolver=resolver, - validator=DependencyValidator(), + validator=AllValidator(validators=default_validators()), ) return _get_kernel_layer(self, kernel) diff --git a/kernels/src/kernels/load.py b/kernels/src/kernels/load.py index e37af7ee..a6ed7193 100644 --- a/kernels/src/kernels/load.py +++ b/kernels/src/kernels/load.py @@ -7,7 +7,13 @@ from kernels_data import KernelDependency, KernelVersion from kernels._versions import revision_or_version -from kernels.deps import AllValidator, ArchValidator, DependencyValidator, Validator, resolve_kernel_tree +from kernels.deps import ( + AllValidator, + ArchValidator, + Validator, + default_validators, + resolve_kernel_tree, +) from kernels.hf_hub import _get_hf_api from kernels.locking import ( get_caller_locked_kernel_revision, @@ -156,7 +162,7 @@ def get_kernel( ), ] - validators: list[Validator] = [DependencyValidator()] + validators: list[Validator] = default_validators() if check_arch: validators.append(ArchValidator()) @@ -219,7 +225,7 @@ def get_local_kernel( # We don't have a name for the kernel, so let's just use the path. kernel=KernelDependency(repo_id=str(repo_path), version=KernelVersion.Version(0)), resolver=SequentialResolver(resolvers), - validator=DependencyValidator(), + validator=AllValidator(validators=default_validators()), ) @@ -336,7 +342,7 @@ def load_kernel( backend=backend, kernel=kernel_dep, resolver=resolver, - validator=DependencyValidator(), + validator=AllValidator(validators=default_validators()), ) @@ -377,5 +383,5 @@ def get_locked_kernel( backend=None, kernel=kernel_dep, resolver=resolver, - validator=DependencyValidator(), + validator=AllValidator(validators=default_validators()), ) diff --git a/kernels/tests/test_minver.py b/kernels/tests/test_minver.py index c7a75ff9..7b1ff995 100644 --- a/kernels/tests/test_minver.py +++ b/kernels/tests/test_minver.py @@ -1,22 +1,17 @@ import json import re +from pathlib import Path import pytest from kernels_data import Metadata, Version import kernels -from kernels.importer import ( - _import_from_path, - _installed_version, - _loaded_kernels, - _warn_if_below_minver, -) +from kernels.deps import DepTreeNode, MinverValidator, _installed_version +from kernels.resolver import LocalKernel -def _write_variant(tmp_path, minver): - variant_dir = tmp_path / "build" / "torch28-cxx11-cu128-x86_64-linux" - variant_dir.mkdir(parents=True) - metadata = { +def _make_tree(minver): + metadata_dict = { "id": "activation_1_cuda", "name": "activation", "version": 1, @@ -25,27 +20,22 @@ def _write_variant(tmp_path, minver): "backend": {"type": "cuda"}, } if minver is not None: - metadata["kernels-minver"] = minver - (variant_dir / "metadata.json").write_text(json.dumps(metadata)) - return variant_dir + metadata_dict["kernels-minver"] = minver + metadata = Metadata.from_bytes(json.dumps(metadata_dict).encode("utf-8")) + variant_path = Path("build") / "torch28-cxx11-cu128-x86_64-linux" + return DepTreeNode(location=LocalKernel(variant_path, metadata), deps={}) @pytest.mark.parametrize("minver", [None, "0.0.1"]) -def test_no_warning_when_minver_met(tmp_path, recwarn, minver): - variant_dir = _write_variant(tmp_path, minver) - metadata = Metadata.read_from_file(variant_dir / "metadata.json") - _warn_if_below_minver(metadata, variant_dir.name) - assert len(recwarn) == 0 +def test_no_error_when_minver_met(minver): + MinverValidator().validate(tree=_make_tree(minver)) -def test_no_warning_for_dev_version_of_required_release(tmp_path, recwarn, monkeypatch): +def test_no_error_for_dev_version_of_required_release(monkeypatch): # A development version implements the release it leads up to, so # `0.17.0.dev0` must satisfy a `0.17.0` requirement. monkeypatch.setattr(kernels, "__version__", "0.17.0.dev0") - variant_dir = _write_variant(tmp_path, "0.17.0") - metadata = Metadata.read_from_file(variant_dir / "metadata.json") - _warn_if_below_minver(metadata, variant_dir.name) - assert len(recwarn) == 0 + MinverValidator().validate(tree=_make_tree("0.17.0")) @pytest.mark.parametrize( @@ -62,14 +52,11 @@ def test_installed_version_is_none_for_non_pep440_version(monkeypatch): assert _installed_version() is None -def test_unparseable_installed_version_does_not_fail_load(tmp_path, recwarn, monkeypatch): - # A version that cannot be compared must not turn this advisory check into - # a hard failure. +def test_unparseable_installed_version_does_not_fail_validation(monkeypatch): + # A version that cannot be compared must not turn this check into a + # failure. monkeypatch.setattr(kernels, "__version__", "0.17.0-dirty") - variant_dir = _write_variant(tmp_path, "999.1.0") - metadata = Metadata.read_from_file(variant_dir / "metadata.json") - _warn_if_below_minver(metadata, variant_dir.name) - assert len(recwarn) == 0 + MinverValidator().validate(tree=_make_tree("999.1.0")) def test_version_ordering_is_numeric_not_lexicographic(): @@ -80,27 +67,11 @@ def test_version_ordering_is_numeric_not_lexicographic(): assert Version.from_str("0.14.0") < Version.from_str("0.14.1") -def test_warns_when_minver_not_met(tmp_path): - variant_dir = _write_variant(tmp_path, "999.1.0") - metadata = Metadata.read_from_file(variant_dir / "metadata.json") - with pytest.warns(UserWarning, match="requires kernels>=999.1"): - _warn_if_below_minver(metadata, variant_dir.name) - - -def test_warning_mentions_installed_version(tmp_path): - variant_dir = _write_variant(tmp_path, "999.1.0") - metadata = Metadata.read_from_file(variant_dir / "metadata.json") - with pytest.warns(UserWarning, match=f"version {re.escape(kernels.__version__)} is installed"): - _warn_if_below_minver(metadata, variant_dir.name) +def test_raises_when_minver_not_met(): + with pytest.raises(RuntimeError, match="requires kernels>=999.1"): + MinverValidator().validate(tree=_make_tree("999.1.0")) -def test_import_from_path_warns_on_unmet_minver(tmp_path): - variant_dir = _write_variant(tmp_path, "999.1.0") - (variant_dir / "__init__.py").write_text("value = 42\n") - _loaded_kernels.pop(variant_dir, None) - try: - with pytest.warns(UserWarning, match="requires kernels>=999.1"): - module = _import_from_path(variant_dir, deps={}) - assert module.value == 42 - finally: - _loaded_kernels.pop(variant_dir, None) +def test_error_mentions_installed_version(): + with pytest.raises(RuntimeError, match=f"version {re.escape(kernels.__version__)} is installed"): + MinverValidator().validate(tree=_make_tree("999.1.0")) diff --git a/nix-builder/pkgs/get-kernel-check/get-kernel-check-hook.py b/nix-builder/pkgs/get-kernel-check/get-kernel-check-hook.py index 1b8e7488..2b2b720b 100755 --- a/nix-builder/pkgs/get-kernel-check/get-kernel-check-hook.py +++ b/nix-builder/pkgs/get-kernel-check/get-kernel-check-hook.py @@ -1,7 +1,7 @@ import os from pathlib import Path -from kernels.deps import DependencyValidator +from kernels.deps import AllValidator, default_validators from kernels.hf_hub import _get_hf_api from kernels.load import get_kernel_with_resolver from kernels.resolver import KernelPathsResolver, RepoPathsResolver, SequentialResolver @@ -30,5 +30,5 @@ backend=None, kernel=kernel, resolver=SequentialResolver(resolvers=resolvers), - validator=DependencyValidator(), + validator=AllValidator(validators=default_validators()), )