mirror of
https://github.com/lancedb/lancedb.git
synced 2026-11-13 14:16:54 +00:00
fix(python): warn when Functions code packages omit resource files (#4804)
Packages passed through `@udf(code=[...])` silently omit non-Python resources, so registration can appear successful before a worker fails to open a data file. Warn during function preparation with the omitted paths and document the source-only boundary next to `code=`. The diagnostic explains how to install a distribution that includes the data and remove the package from `code=` to avoid shadowing that installation. Python cache files do not produce warnings. Excluded directories are reported at their boundary, preserving the existing traversal and Python-source artifact format. This implements the warning-and-documentation alternative in ENT-3492; it does not add resource uploads. On EC2, the regression fails on baseline `f9389f113` because no warning is emitted; all 60 packaging tests pass at `e23445b96`, using the matching native extension built from unchanged Rust sources. CI-scoped Python lint with Ruff 0.9.9 passes. Whole-site Mintlify validation reports the same four snippet-import warnings on the baseline and this head; repository-root Ruff likewise reports the same 48 findings under `docs/web-tests`. These checks are not clean passes, but their failures do not involve this change. Fixes ENT-3492
This commit is contained in:
@@ -1764,7 +1764,12 @@ class _SourcePackager:
|
||||
|
||||
def _code_files(modules: Sequence[Any]) -> tuple[dict[str, str], frozenset[str]]:
|
||||
"""The Python source files of the modules and packages in ``code=``,
|
||||
keyed by their path on the Function's import path."""
|
||||
keyed by their path on the Function's import path.
|
||||
|
||||
Package resources are not part of the source artifact. Warn about files
|
||||
and excluded directories that the walk leaves behind instead of silently
|
||||
producing a Function that will fail later on the worker.
|
||||
"""
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
@@ -1800,19 +1805,39 @@ def _code_files(modules: Sequence[Any]) -> tuple[dict[str, str], frozenset[str]]
|
||||
)
|
||||
root = Path(paths[0])
|
||||
sources = []
|
||||
skipped = set()
|
||||
for directory, subdirectories, filenames in os.walk(root):
|
||||
# Only importable modules travel: data files, caches, and
|
||||
# directories that are not identifiers cannot be imported.
|
||||
subdirectories[:] = sorted(
|
||||
child
|
||||
for child in subdirectories
|
||||
if _SOURCE_PATH_COMPONENT.fullmatch(child)
|
||||
and child != "__pycache__"
|
||||
)
|
||||
for filename in filenames:
|
||||
kept = []
|
||||
for child in sorted(subdirectories):
|
||||
if child == "__pycache__":
|
||||
continue
|
||||
if _SOURCE_PATH_COMPONENT.fullmatch(child):
|
||||
kept.append(child)
|
||||
else:
|
||||
relative = (Path(directory) / child).relative_to(root)
|
||||
skipped.add(f"{name}/{relative.as_posix()}/")
|
||||
subdirectories[:] = kept
|
||||
for filename in sorted(filenames):
|
||||
stem, suffix = os.path.splitext(filename)
|
||||
if suffix in {".pyc", ".pyo"}:
|
||||
continue
|
||||
if suffix == ".py" and _SOURCE_PATH_COMPONENT.fullmatch(stem):
|
||||
sources.append(Path(directory) / filename)
|
||||
else:
|
||||
relative = (Path(directory) / filename).relative_to(root)
|
||||
skipped.add(f"{name}/{relative.as_posix()}")
|
||||
if skipped:
|
||||
skipped_paths = ", ".join(sorted(skipped))
|
||||
warnings.warn(
|
||||
f"code= ships Python .py modules only; package {name!r} "
|
||||
f"will not include: {skipped_paths}. Install a package that "
|
||||
"includes the required resources with pip= or conda=, and "
|
||||
f"remove {name!r} from code= so the copied package cannot "
|
||||
"shadow the installed distribution.",
|
||||
UserWarning,
|
||||
)
|
||||
for path in sources:
|
||||
relative = path.relative_to(root).as_posix()
|
||||
files[f"{name}/{relative}"] = _source_text(path)
|
||||
@@ -2320,7 +2345,12 @@ def udf(
|
||||
selected through ``refresh_column`` instead of registration.
|
||||
code : sequence of module, optional
|
||||
Top-level modules or packages whose importable ``.py`` files ship
|
||||
with the Function and are importable by their own names.
|
||||
with the Function and are importable by their own names. Package data
|
||||
files are not copied. To use a package that includes required data
|
||||
resources, install it with ``pip=`` or ``conda=`` and remove that
|
||||
package from ``code=`` so the copied Python package cannot shadow the
|
||||
installed distribution. ``@udf`` warns when it skips files from a
|
||||
package in ``code=``.
|
||||
|
||||
Returns
|
||||
-------
|
||||
|
||||
@@ -391,7 +391,21 @@ def test_code_packages_ship_as_a_bundle_and_version_with_their_source(
|
||||
importlib.invalidate_caches()
|
||||
return importlib.import_module("fx_helpers_functions").limited
|
||||
|
||||
first = define()
|
||||
with warnings.catch_warnings(record=True) as caught:
|
||||
warnings.simplefilter("always")
|
||||
first = define()
|
||||
assert len(caught) == 1
|
||||
warning = str(caught[0].message)
|
||||
assert "fx_helpers/fixtures/data.json" in warning
|
||||
assert "fx_helpers/not-a-module.py" in warning
|
||||
assert warning.index("fx_helpers/fixtures/data.json") < warning.index(
|
||||
"fx_helpers/not-a-module.py"
|
||||
)
|
||||
assert "remove 'fx_helpers' from code=" in warning
|
||||
assert "pip=" in warning
|
||||
assert "conda=" in warning
|
||||
assert "__pycache__" not in warning
|
||||
assert ".pyc" not in warning
|
||||
shipped = bundle_files(first)
|
||||
assert sorted(shipped) == [
|
||||
"fx_helpers/__init__.py",
|
||||
@@ -407,7 +421,10 @@ def test_code_packages_ship_as_a_bundle_and_version_with_their_source(
|
||||
helper_package(
|
||||
"fx_helpers", {**files, "rate.py": textwrap.dedent(files["rate.py"]) + "# v2\n"}
|
||||
)
|
||||
second = define()
|
||||
with warnings.catch_warnings(record=True) as caught:
|
||||
warnings.simplefilter("always")
|
||||
second = define()
|
||||
assert len(caught) == 1
|
||||
assert (
|
||||
first.registration_request.artifact.digest
|
||||
!= second.registration_request.artifact.digest
|
||||
@@ -415,6 +432,57 @@ def test_code_packages_ship_as_a_bundle_and_version_with_their_source(
|
||||
assert first.registration_request.runtime == second.registration_request.runtime
|
||||
|
||||
|
||||
def test_code_package_cache_artifacts_do_not_warn(helper_package):
|
||||
cache = helper_package(
|
||||
"fx_cache",
|
||||
{
|
||||
"__init__.py": "",
|
||||
"__pycache__/ignored.cpython-312.pyc": "",
|
||||
"stale.pyo": "",
|
||||
},
|
||||
)
|
||||
|
||||
with warnings.catch_warnings(record=True) as caught:
|
||||
warnings.simplefilter("always")
|
||||
|
||||
@udf(code=[cache])
|
||||
def cache_only(value: int) -> int:
|
||||
return value
|
||||
|
||||
assert not caught
|
||||
assert sorted(bundle_files(cache_only)) == [
|
||||
"fx_cache/__init__.py",
|
||||
"user_function.py",
|
||||
]
|
||||
|
||||
|
||||
def test_code_package_nonidentifier_resource_dir_warns_at_boundary(helper_package):
|
||||
resources = helper_package(
|
||||
"fx_resources",
|
||||
{
|
||||
"__init__.py": "",
|
||||
"label-maps/labels.json": "{}",
|
||||
},
|
||||
)
|
||||
|
||||
with warnings.catch_warnings(record=True) as caught:
|
||||
warnings.simplefilter("always")
|
||||
|
||||
@udf(code=[resources])
|
||||
def resource_boundary(value: int) -> int:
|
||||
return value
|
||||
|
||||
assert len(caught) == 1
|
||||
warning = str(caught[0].message)
|
||||
assert "fx_resources/label-maps/" in warning
|
||||
assert "remove 'fx_resources' from code=" in warning
|
||||
assert "labels.json" not in warning
|
||||
assert sorted(bundle_files(resource_boundary)) == [
|
||||
"fx_resources/__init__.py",
|
||||
"user_function.py",
|
||||
]
|
||||
|
||||
|
||||
def test_code_rejects_submodules_duplicates_and_non_modules(helper_package):
|
||||
helpers = helper_package("fx_rejects", {"__init__.py": "", "sub.py": ""})
|
||||
import fx_rejects.sub
|
||||
|
||||
Reference in New Issue
Block a user