fix: classify user-package import failures in load_python_modules

`load_python_modules` has three `importlib.import_module(<user-module>)` call
sites. #1168 gave two of them the broad "anything raised inside a user module is
a user error" treatment, but the third — the branch that imports a *directory*
as a package when it contains no top-level `.py` files — kept its original
narrow handler:

    except (ValueError, ModuleNotFoundError):
        pass

That leaks in both directions:

1. Any other exception from the user's `__init__.py` (`TypeError`, `OSError`,
   `SyntaxError`, `PydanticUserError`, yaml `ScannerError`, ...) escapes
   `load_python_modules` raw and crash-reports to Sentry, even though it is by
   definition a bug in the user's code or one of its third-party imports. This
   is the same class the Sentry corpus shows for the other two sites
   (FLYTE-SDK-39 and friends), which #1168 already closed.

2. A `ModuleNotFoundError` raised from *inside* the package — a dependency the
   user has not installed — is swallowed by the bare `pass`. Deploy then
   proceeds with zero environments and no explanation. The silent half is
   arguably worse than the crash.

The narrow catch was not wrong when it was written: it is control flow for
"this directory is not an importable package, skip it". This keeps that, and
only that:

- `ValueError` is now scoped to the `relative_to` call that actually raises it,
  so a `ValueError` from user code at import time is no longer mistaken for a
  path-outside-the-root.
- `ModuleNotFoundError` is skipped only when it names the target module (or a
  parent package of it) — i.e. the directory genuinely is not importable here.
  `ModuleNotFoundError.name` distinguishes that from a missing inner import.
- Everything else is recorded in `failed_paths`, consistent with the sibling
  directory branch, so `flyte deploy` surfaces a clean `ClickException` and
  honours `--ignore-load-errors` instead of crash-reporting.

No Sentry issue is currently firing on this branch, so nothing is claimed as
fixed — this closes the site #1168 missed, plus the silent-skip bug next to it.

Six tests; the two that pin the leak and the silent skip fail on main, and the
four that pin the preserved skip-and-continue behaviour pass on both sides by
design.

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Author: EngHabu

src/flyte/_utils/module_loader.py

@@ -13,6 +13,21 @@
 from flyte._logging import logger
 
 
+def _names_missing_module(err: ModuleNotFoundError, mod: str) -> bool:
+    """Whether `err` reports that `mod` itself -- rather than one of its imports -- is absent.
+
+    `importlib.import_module(mod)` raises `ModuleNotFoundError` both when `mod` does not exist
+    and when `mod` imports something that does not exist. Only the first means "this directory
+    is not an importable package"; the second is a user dependency error worth surfacing.
+    `ModuleNotFoundError.name` distinguishes them.
+    """
+    name = err.name
+    if not name:
+        # No name to compare against, so keep the historical skip-and-continue behaviour.
+        return True
+    return mod == name or mod.startswith(f"{name}.")
+
+
 def _relative_to_root(path: Path, root_dir: Path) -> Path:
     """Resolve `path` relative to `root_dir` and translate `ValueError` into a clear ClickException.
 
@@ -92,11 +107,31 @@ def load_python_modules(
             # If no .py files found, try importing as a module
             try:
                 rel_path = path.resolve().relative_to(root_dir)
+            except ValueError:
+                # Outside the root, so there is no module name to import it under.
+                rel_path = None
+            if rel_path is not None:
                 mod = ".".join(rel_path.parts)
-                imported_module = importlib.import_module(mod)
-                loaded_modules.append(imported_module)
-            except (ValueError, ModuleNotFoundError):
-                pass
+                try:
+                    imported_module = importlib.import_module(mod)
+                    loaded_modules.append(imported_module)
+                except flyte.errors.ModuleLoadError as e:
+                    failed_paths.append((path, str(e)))
+                except ModuleNotFoundError as e:
+                    # `mod` itself not being importable just means this directory is not a
+                    # package, which is the case this branch has always skipped. A module
+                    # missing from *inside* the package's `__init__.py` is a different thing
+                    # -- a user dependency error -- and swallowing it silently deploys zero
+                    # environments with no explanation.
+                    if not _names_missing_module(e, mod):
+                        failed_paths.append((path, f"{type(e).__name__}: {e}"))
+                except Exception as e:
+                    # Anything else raised inside `importlib.import_module(<user-package>)` is an
+                    # error in the user's `__init__.py` or one of its third-party imports, not an
+                    # SDK bug. Record it as a load failure (consistent with the .py-file branch
+                    # below) so deploy surfaces a clean message via `--ignore-load-errors`
+                    # instead of crash-reporting to Sentry.
+                    failed_paths.append((path, f"{type(e).__name__}: {e}"))
         else:
             with Progress(
                 TextColumn("[progress.description]{task.description}"),

tests/flyte/utils/test_module_loader.py

@@ -366,3 +366,133 @@ def test_load_python_modules_single_file_wraps_runtime_error(tmp_path):
     assert "workflow.py" in msg
     assert "RuntimeError" in msg
     assert "union.sandbox" in msg
+
+
+# ---------------------------------------------------------------------------
+# Directory-with-no-.py-files branch: `flyte deploy ./somepkg` where the package's
+# code lives in subpackages, so the directory is imported as a module instead.
+# ---------------------------------------------------------------------------
+
+
+def _package_project(tmp_path, name, init_body):
+    """Build `project/<name>/__init__.py` with no top-level .py files under `<name>`.
+
+    Each test uses its own package name so a cached `sys.modules` entry from one test
+    cannot change what another one imports.
+    """
+    root = tmp_path / "project"
+    root.mkdir()
+    pkg = root / name
+    pkg.mkdir()
+    (pkg / "__init__.py").write_text(init_body)
+    return root, pkg
+
+
+def test_load_python_modules_package_records_user_init_errors(tmp_path):
+    """An exception raised by the package's own __init__.py is a user-code bug.
+
+    It used to escape `load_python_modules` raw -- the branch only caught
+    (ValueError, ModuleNotFoundError) -- and crash-reported to Sentry."""
+    root, pkg = _package_project(tmp_path, "pkg_init_raises", "raise TypeError('user init blew up')\n")
+
+    sys.path.insert(0, str(root))
+    try:
+        modules, failed = load_python_modules(pkg, root_dir=root, recursive=False)
+    finally:
+        sys.path.remove(str(root))
+        sys.modules.pop("pkg_init_raises", None)
+
+    assert modules == []
+    assert len(failed) == 1
+    failed_path, failed_msg = failed[0]
+    assert failed_path == pkg
+    assert "TypeError" in failed_msg
+    assert "user init blew up" in failed_msg
+
+
+def test_load_python_modules_package_records_missing_dependency(tmp_path):
+    """A dependency missing from inside the package's __init__.py is a user error.
+
+    It used to be swallowed by the bare `except ModuleNotFoundError: pass`, so deploy
+    silently proceeded with zero environments and no explanation."""
+    root, pkg = _package_project(tmp_path, "pkg_missing_dep", "import totally_missing_dep_xyz\n")
+
+    sys.path.insert(0, str(root))
+    try:
+        modules, failed = load_python_modules(pkg, root_dir=root, recursive=False)
+    finally:
+        sys.path.remove(str(root))
+        sys.modules.pop("pkg_missing_dep", None)
+
+    assert modules == []
+    assert len(failed) == 1
+    failed_path, failed_msg = failed[0]
+    assert failed_path == pkg
+    assert "totally_missing_dep_xyz" in failed_msg
+
+
+def test_load_python_modules_package_skips_when_not_importable(tmp_path):
+    """The directory simply not being an importable package stays a silent skip.
+
+    This is the case the original `except ModuleNotFoundError: pass` existed for and
+    it must keep working -- only errors from *inside* the package are now surfaced."""
+    root, pkg = _package_project(tmp_path, "pkg_not_importable", "VALUE = 1\n")
+
+    # `root` is never put on sys.path, so the package itself cannot be imported.
+    def fake_import(mod_name):
+        raise ModuleNotFoundError(f"No module named '{mod_name}'", name=mod_name)
+
+    with patch("importlib.import_module", side_effect=fake_import):
+        modules, failed = load_python_modules(pkg, root_dir=root, recursive=False)
+
+    assert modules == []
+    assert failed == []
+
+
+def test_load_python_modules_package_skips_parent_package_missing(tmp_path):
+    """A missing *parent* of the target also means "not importable here" -- still a skip."""
+    root = tmp_path / "project"
+    root.mkdir()
+    pkg = root / "outer" / "inner"
+    pkg.mkdir(parents=True)
+    (pkg / "__init__.py").write_text("VALUE = 1\n")
+
+    def fake_import(mod_name):
+        assert mod_name == "outer.inner"
+        raise ModuleNotFoundError("No module named 'outer'", name="outer")
+
+    with patch("importlib.import_module", side_effect=fake_import):
+        modules, failed = load_python_modules(pkg, root_dir=root, recursive=False)
+
+    assert modules == []
+    assert failed == []
+
+
+def test_load_python_modules_package_outside_root_is_skipped(tmp_path):
+    """A package directory outside the root has no module name, so it is skipped."""
+    root = tmp_path / "project"
+    root.mkdir()
+    other = tmp_path / "elsewhere" / "pkg_outside_root"
+    other.mkdir(parents=True)
+    (other / "__init__.py").write_text("VALUE = 1\n")
+
+    modules, failed = load_python_modules(other, root_dir=root, recursive=False)
+
+    assert modules == []
+    assert failed == []
+
+
+def test_load_python_modules_package_loads_healthy_package(tmp_path):
+    """The happy path is unchanged: an importable package is loaded."""
+    root, pkg = _package_project(tmp_path, "pkg_healthy", "VALUE = 1\n")
+
+    sys.path.insert(0, str(root))
+    try:
+        modules, failed = load_python_modules(pkg, root_dir=root, recursive=False)
+    finally:
+        sys.path.remove(str(root))
+        sys.modules.pop("pkg_healthy", None)
+
+    assert failed == []
+    assert len(modules) == 1
+    assert modules[0].VALUE == 1