Skip to content

Commit 2bff0eb

Browse files
authored
Fix recursive local module discovery in bundle serialization (#19124)
1 parent 18000dd commit 2bff0eb

File tree

3 files changed

+133
-32
lines changed

3 files changed

+133
-32
lines changed

AGENTS.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ prefect config view # Inspect configuration
7474

7575
- GitHub issues are used for tracking issues (use the `gh` cli)
7676
- Pre-commit hooks required (never use `--no-verify`)
77-
- There are some slower pre-push hooks that may modify files on `git push`; when that happens, run `git commit --amend` to bring those into the prior commit (never use `--amend` in any other situation unless asked)
7877
- Dependencies: updates to client-side deps in `@pyproject.toml` require parallel changes ing `@client/pyproject.toml`
7978
- AGENTS.md always symlinked to CLAUDE.md
8079
- the redis lease storage lives in @src/integrations/prefect-redis/

src/prefect/_experimental/bundles/__init__.py

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -209,28 +209,49 @@ def _discover_local_dependencies(
209209

210210
module_name = flow_module.__name__
211211

212+
# Process the flow's module and all its dependencies recursively
213+
_process_module_dependencies(flow_module, module_name, local_modules, visited)
214+
215+
return local_modules
216+
217+
218+
def _process_module_dependencies(
219+
module: ModuleType,
220+
module_name: str,
221+
local_modules: set[str],
222+
visited: set[str],
223+
) -> None:
224+
"""
225+
Recursively process a module and discover its local dependencies.
226+
227+
Args:
228+
module: The module to process.
229+
module_name: The name of the module.
230+
local_modules: Set to accumulate discovered local modules.
231+
visited: Set of already visited modules to avoid infinite recursion.
232+
"""
212233
# Skip if we've already processed this module
213234
if module_name in visited:
214-
return local_modules
235+
return
215236
visited.add(module_name)
216237

217-
# Check if the flow's module itself is local
218-
module_file = getattr(flow_module, "__file__", None)
238+
# Check if this is a local module
239+
module_file = getattr(module, "__file__", None)
219240
if not module_file or not _is_local_module(module_name, module_file):
220-
return local_modules
241+
return
221242

222243
local_modules.add(module_name)
223244

224245
# Get the source code of the module
225246
try:
226-
source_code = inspect.getsource(flow_module)
247+
source_code = inspect.getsource(module)
227248
except (OSError, TypeError):
228-
# Can't get source for the flow's module
229-
return local_modules
249+
# Can't get source for this module
250+
return
230251

231252
imports = _extract_imports_from_source(source_code)
232253

233-
# Check each import to see if it's local
254+
# Check each import to see if it's local and recursively process it
234255
for import_name in imports:
235256
# Skip if already visited
236257
if import_name in visited:
@@ -249,30 +270,13 @@ def _discover_local_dependencies(
249270
else:
250271
imported_module = importlib.import_module(import_name)
251272
except (ImportError, AttributeError):
252-
# Can't import or module has no __file__, skip it
253-
continue
254-
255-
imported_file = getattr(imported_module, "__file__", None)
256-
if not imported_file or not _is_local_module(import_name, imported_file):
273+
# Can't import, skip it
257274
continue
258275

259-
local_modules.add(import_name)
260-
visited.add(import_name)
261-
262-
# Recursively check this module's dependencies
263-
try:
264-
imported_source = inspect.getsource(imported_module)
265-
except (OSError, TypeError):
266-
# Can't get source for this module, skip its dependencies
267-
continue
268-
269-
nested_imports = _extract_imports_from_source(imported_source)
270-
for nested_import in nested_imports:
271-
if nested_import not in visited and _is_local_module(nested_import):
272-
local_modules.add(nested_import)
273-
visited.add(nested_import)
274-
275-
return local_modules
276+
# Recursively process this imported module
277+
_process_module_dependencies(
278+
imported_module, import_name, local_modules, visited
279+
)
276280

277281

278282
@contextmanager

tests/experimental/test_bundles.py

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import signal
33
import subprocess
44
import sys
5+
from pathlib import Path
56
from typing import Any, Literal
67
from unittest.mock import MagicMock, patch
78

@@ -490,7 +491,9 @@ def mock_unregister(module):
490491
assert len(unregistered) == 1
491492
assert unregistered[0] == mock_module
492493

493-
def test_pickle_local_modules_handles_import_errors(self, caplog):
494+
def test_pickle_local_modules_handles_import_errors(
495+
self, caplog: pytest.LogCaptureFixture
496+
):
494497
"""Test that import errors are handled gracefully."""
495498

496499
@flow
@@ -506,3 +509,98 @@ def test_flow():
506509

507510
# Check that a debug message was logged about the failure
508511
assert "Failed to register module nonexistent_module" in caplog.text
512+
513+
def test_discover_deeply_nested_local_dependencies(self, tmp_path: Path):
514+
"""Test that local dependencies are discovered recursively through multiple levels.
515+
516+
This tests the scenario where:
517+
- flow_module imports module_b
518+
- module_b imports module_c
519+
- module_c imports module_d
520+
521+
All modules should be discovered, including module_d which is 3 levels deep.
522+
"""
523+
# Create temporary package structure with deep nesting
524+
package_root = tmp_path / "test_packages"
525+
package_root.mkdir()
526+
527+
# Create flow_module package
528+
flow_pkg = package_root / "flow_module"
529+
flow_pkg.mkdir()
530+
(flow_pkg / "__init__.py").write_text("")
531+
532+
# Create module_b package
533+
module_b_pkg = package_root / "module_b"
534+
module_b_pkg.mkdir()
535+
(module_b_pkg / "__init__.py").write_text("")
536+
537+
# Create module_c package
538+
module_c_pkg = package_root / "module_c"
539+
module_c_pkg.mkdir()
540+
(module_c_pkg / "__init__.py").write_text("")
541+
542+
# Create module_d package (deepest level)
543+
module_d_pkg = package_root / "module_d"
544+
module_d_pkg.mkdir()
545+
(module_d_pkg / "__init__.py").write_text("")
546+
547+
# Create module_d with a simple function
548+
(module_d_pkg / "utils.py").write_text("""
549+
def function_d():
550+
return "d"
551+
""")
552+
553+
# Create module_c that imports from module_d
554+
(module_c_pkg / "utils.py").write_text("""
555+
from module_d.utils import function_d
556+
557+
def function_c():
558+
return function_d()
559+
""")
560+
561+
# Create module_b that imports from module_c
562+
(module_b_pkg / "utils.py").write_text("""
563+
from module_c.utils import function_c
564+
565+
def function_b():
566+
return function_c()
567+
""")
568+
569+
# Create flow_module that imports from module_b
570+
(flow_pkg / "my_flow.py").write_text("""
571+
from module_b.utils import function_b
572+
from prefect import flow
573+
574+
@flow
575+
def test_flow():
576+
return function_b()
577+
""")
578+
579+
# Add package_root to sys.path so modules can be imported
580+
sys.path.insert(0, str(package_root))
581+
582+
try:
583+
# Import the flow module and get the flow
584+
import flow_module.my_flow
585+
586+
flow_obj = flow_module.my_flow.test_flow
587+
588+
# Discover dependencies
589+
deps = _discover_local_dependencies(flow_obj)
590+
591+
# All four modules should be discovered
592+
assert "flow_module.my_flow" in deps, (
593+
"Flow module itself should be discovered"
594+
)
595+
assert "module_b.utils" in deps, "First-level import should be discovered"
596+
assert "module_c.utils" in deps, "Second-level import should be discovered"
597+
assert "module_d.utils" in deps, "Third-level import should be discovered"
598+
599+
finally:
600+
# Clean up sys.path and sys.modules
601+
sys.path.remove(str(package_root))
602+
for module in list(sys.modules.keys()):
603+
if module.startswith(
604+
("flow_module", "module_b", "module_c", "module_d")
605+
):
606+
del sys.modules[module]

0 commit comments

Comments
 (0)