Skip to content

Commit 44ad42c

Browse files
committed
fix(cli): add pre-deployment validation for agent module imports
Add pre-deployment validation to `adk deploy agent_engine` that catches import errors before deployment. This provides clearer error messages and prevents deployments that would fail at runtime. The validation: - Checks that agent.py exists and can be imported - Verifies the expected export (root_agent or app) is present - Provides specific guidance for BaseLlm-related import errors - Properly cleans up sys.path and sys.modules after validation Note: The cloudpickle.register_pickle_by_value() workaround has been removed as Agent Engine now uses source-based deployment via source_packages, which uploads the full source code directly. This makes pickle-based serialization unnecessary for custom classes. Fixes #4208
1 parent 4ee125a commit 44ad42c

File tree

2 files changed

+255
-2
lines changed

2 files changed

+255
-2
lines changed

src/google/adk/cli/cli_deploy.py

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@
1414
from __future__ import annotations
1515

1616
from datetime import datetime
17+
import importlib.util
1718
import json
19+
import logging
1820
import os
1921
import shutil
2022
import subprocess
23+
import sys
24+
import traceback
2125
from typing import Final
2226
from typing import Optional
2327
import warnings
2428

2529
import click
2630
from packaging.version import parse
2731

32+
logger = logging.getLogger('google_adk.' + __name__)
33+
2834
_IS_WINDOWS = os.name == 'nt'
2935
_GCLOUD_CMD = 'gcloud.cmd' if _IS_WINDOWS else 'gcloud'
3036
_LOCAL_STORAGE_FLAG_MIN_VERSION: Final[str] = '1.21.0'
@@ -465,6 +471,120 @@ def _validate_gcloud_extra_args(
465471
)
466472

467473

474+
def _validate_agent_import(
475+
agent_src_path: str,
476+
adk_app_object: str,
477+
is_config_agent: bool,
478+
) -> None:
479+
"""Validates that the agent module can be imported successfully.
480+
481+
This pre-deployment validation catches common issues like missing
482+
dependencies or import errors in custom BaseLlm implementations before
483+
the agent is deployed to Agent Engine. This provides clearer error
484+
messages and prevents deployments that would fail at runtime.
485+
486+
Args:
487+
agent_src_path: Path to the staged agent source code.
488+
adk_app_object: The Python object name to import ('root_agent' or 'app').
489+
is_config_agent: Whether this is a config-based agent.
490+
491+
Raises:
492+
click.ClickException: If the agent module cannot be imported.
493+
"""
494+
if is_config_agent:
495+
# Config agents are loaded from YAML, skip Python import validation
496+
return
497+
498+
agent_module_path = os.path.join(agent_src_path, 'agent.py')
499+
if not os.path.exists(agent_module_path):
500+
raise click.ClickException(
501+
f'Agent module not found at {agent_module_path}. '
502+
'Please ensure your agent folder contains an agent.py file.'
503+
)
504+
505+
# Add the parent directory to sys.path temporarily for import resolution
506+
parent_dir = os.path.dirname(agent_src_path)
507+
module_name = os.path.basename(agent_src_path)
508+
509+
original_sys_path = sys.path.copy()
510+
try:
511+
# Add parent directory to path so imports work correctly
512+
if parent_dir not in sys.path:
513+
sys.path.insert(0, parent_dir)
514+
515+
# Load the agent module spec
516+
spec = importlib.util.spec_from_file_location(
517+
f'{module_name}.agent',
518+
agent_module_path,
519+
submodule_search_locations=[agent_src_path],
520+
)
521+
if spec is None or spec.loader is None:
522+
raise click.ClickException(
523+
f'Failed to load module spec from {agent_module_path}'
524+
)
525+
526+
# Try to load the module
527+
module = importlib.util.module_from_spec(spec)
528+
sys.modules[f'{module_name}.agent'] = module
529+
530+
try:
531+
spec.loader.exec_module(module)
532+
except ImportError as e:
533+
error_msg = str(e)
534+
tb = traceback.format_exc()
535+
536+
# Check for common issues
537+
if 'BaseLlm' in tb or 'base_llm' in tb.lower():
538+
raise click.ClickException(
539+
'Failed to import agent module due to a BaseLlm-related error:\n'
540+
f'{error_msg}\n\n'
541+
'This error often occurs when deploying agents with custom LLM '
542+
'implementations. Please ensure:\n'
543+
'1. All custom LLM classes are defined in files within your agent '
544+
'folder\n'
545+
'2. All required dependencies are listed in requirements.txt\n'
546+
'3. Import paths use relative imports (e.g., "from .my_llm import '
547+
'MyLlm")\n'
548+
'4. Your custom BaseLlm implementation is serializable'
549+
) from e
550+
else:
551+
raise click.ClickException(
552+
f'Failed to import agent module:\n{error_msg}\n\n'
553+
'Please ensure all dependencies are listed in requirements.txt '
554+
'and all imports are resolvable.\n\n'
555+
f'Full traceback:\n{tb}'
556+
) from e
557+
except Exception as e:
558+
tb = traceback.format_exc()
559+
raise click.ClickException(
560+
f'Error while loading agent module:\n{e}\n\n'
561+
'Please check your agent code for errors.\n\n'
562+
f'Full traceback:\n{tb}'
563+
) from e
564+
565+
# Check that the expected object exists
566+
if not hasattr(module, adk_app_object):
567+
available_attrs = [
568+
attr for attr in dir(module) if not attr.startswith('_')
569+
]
570+
raise click.ClickException(
571+
f"Agent module does not export '{adk_app_object}'. "
572+
f'Available exports: {available_attrs}\n\n'
573+
'Please ensure your agent.py exports either "root_agent" or "app".'
574+
)
575+
576+
click.echo(
577+
'Agent module validation successful: '
578+
f'found "{adk_app_object}" in agent.py'
579+
)
580+
581+
finally:
582+
# Restore original sys.path
583+
sys.path[:] = original_sys_path
584+
# Clean up the module from sys.modules
585+
sys.modules.pop(f'{module_name}.agent', None)
586+
587+
468588
def _get_service_option_by_adk_version(
469589
adk_version: str,
470590
session_uri: Optional[str],
@@ -953,6 +1073,10 @@ def to_agent_engine(
9531073
click.echo(f'Config agent detected: {config_root_agent_file}')
9541074
is_config_agent = True
9551075

1076+
# Validate that the agent module can be imported before deployment
1077+
click.echo('Validating agent module...')
1078+
_validate_agent_import(agent_src_path, adk_app_object, is_config_agent)
1079+
9561080
adk_app_file = os.path.join(temp_folder, f'{adk_app}.py')
9571081
if adk_app_object == 'root_agent':
9581082
adk_app_type = 'agent'

tests/unittests/cli/utils/test_cli_deploy.py

Lines changed: 131 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
"""Tests for utilities in cli_deploy."""
1616

17-
1817
from __future__ import annotations
1918

2019
import importlib
@@ -83,7 +82,9 @@ def agent_dir(tmp_path: Path) -> Callable[[bool, bool], Path]:
8382
def _factory(include_requirements: bool, include_env: bool) -> Path:
8483
base = tmp_path / "agent"
8584
base.mkdir()
86-
(base / "agent.py").write_text("# dummy agent")
85+
(base / "agent.py").write_text(
86+
"# dummy agent\nroot_agent = 'dummy_agent'\n"
87+
)
8788
(base / "__init__.py").touch()
8889
if include_requirements:
8990
(base / "requirements.txt").write_text("pytest\n")
@@ -407,3 +408,131 @@ def mock_subprocess_run(*args, **kwargs):
407408

408409
# 4. Verify cleanup
409410
assert str(rmtree_recorder.get_last_call_args()[0]) == str(tmp_path)
411+
412+
413+
# _validate_agent_import tests
414+
class TestValidateAgentImport:
415+
"""Tests for the _validate_agent_import function."""
416+
417+
def test_skips_config_agents(self, tmp_path: Path) -> None:
418+
"""Config agents should skip validation."""
419+
# This should not raise even with no agent.py file
420+
cli_deploy._validate_agent_import(
421+
str(tmp_path), "root_agent", is_config_agent=True
422+
)
423+
424+
def test_raises_on_missing_agent_module(self, tmp_path: Path) -> None:
425+
"""Should raise when agent.py is missing."""
426+
with pytest.raises(click.ClickException) as exc_info:
427+
cli_deploy._validate_agent_import(
428+
str(tmp_path), "root_agent", is_config_agent=False
429+
)
430+
assert "Agent module not found" in str(exc_info.value)
431+
432+
def test_raises_on_missing_export(self, tmp_path: Path) -> None:
433+
"""Should raise when the expected export is missing."""
434+
agent_file = tmp_path / "agent.py"
435+
agent_file.write_text("some_other_var = 'hello'\n")
436+
(tmp_path / "__init__.py").touch()
437+
438+
with pytest.raises(click.ClickException) as exc_info:
439+
cli_deploy._validate_agent_import(
440+
str(tmp_path), "root_agent", is_config_agent=False
441+
)
442+
assert "does not export 'root_agent'" in str(exc_info.value)
443+
assert "some_other_var" in str(exc_info.value)
444+
445+
def test_success_with_root_agent_export(self, tmp_path: Path) -> None:
446+
"""Should succeed when root_agent is exported."""
447+
agent_file = tmp_path / "agent.py"
448+
agent_file.write_text("root_agent = 'my_agent'\n")
449+
(tmp_path / "__init__.py").touch()
450+
451+
# Should not raise
452+
cli_deploy._validate_agent_import(
453+
str(tmp_path), "root_agent", is_config_agent=False
454+
)
455+
456+
def test_success_with_app_export(self, tmp_path: Path) -> None:
457+
"""Should succeed when app is exported."""
458+
agent_file = tmp_path / "agent.py"
459+
agent_file.write_text("app = 'my_app'\n")
460+
(tmp_path / "__init__.py").touch()
461+
462+
# Should not raise
463+
cli_deploy._validate_agent_import(
464+
str(tmp_path), "app", is_config_agent=False
465+
)
466+
467+
def test_raises_on_import_error(self, tmp_path: Path) -> None:
468+
"""Should raise with helpful message on ImportError."""
469+
agent_file = tmp_path / "agent.py"
470+
agent_file.write_text("from nonexistent_module import something\n")
471+
(tmp_path / "__init__.py").touch()
472+
473+
with pytest.raises(click.ClickException) as exc_info:
474+
cli_deploy._validate_agent_import(
475+
str(tmp_path), "root_agent", is_config_agent=False
476+
)
477+
assert "Failed to import agent module" in str(exc_info.value)
478+
assert "nonexistent_module" in str(exc_info.value)
479+
480+
def test_raises_on_basellm_import_error(self, tmp_path: Path) -> None:
481+
"""Should provide specific guidance for BaseLlm import errors."""
482+
agent_file = tmp_path / "agent.py"
483+
agent_file.write_text(
484+
"from google.adk.models.base_llm import NonexistentBaseLlm\n"
485+
)
486+
(tmp_path / "__init__.py").touch()
487+
488+
with pytest.raises(click.ClickException) as exc_info:
489+
cli_deploy._validate_agent_import(
490+
str(tmp_path), "root_agent", is_config_agent=False
491+
)
492+
assert "BaseLlm-related error" in str(exc_info.value)
493+
assert "custom LLM" in str(exc_info.value)
494+
495+
def test_raises_on_syntax_error(self, tmp_path: Path) -> None:
496+
"""Should raise on syntax errors in agent.py."""
497+
agent_file = tmp_path / "agent.py"
498+
agent_file.write_text("def invalid syntax here:\n")
499+
(tmp_path / "__init__.py").touch()
500+
501+
with pytest.raises(click.ClickException) as exc_info:
502+
cli_deploy._validate_agent_import(
503+
str(tmp_path), "root_agent", is_config_agent=False
504+
)
505+
assert "Error while loading agent module" in str(exc_info.value)
506+
507+
def test_cleans_up_sys_modules(self, tmp_path: Path) -> None:
508+
"""Should clean up sys.modules after validation."""
509+
agent_file = tmp_path / "agent.py"
510+
agent_file.write_text("root_agent = 'my_agent'\n")
511+
(tmp_path / "__init__.py").touch()
512+
513+
module_name = tmp_path.name
514+
agent_module_key = f"{module_name}.agent"
515+
516+
# Ensure module is not in sys.modules before
517+
assert agent_module_key not in sys.modules
518+
519+
cli_deploy._validate_agent_import(
520+
str(tmp_path), "root_agent", is_config_agent=False
521+
)
522+
523+
# Ensure module is cleaned up after
524+
assert agent_module_key not in sys.modules
525+
526+
def test_restores_sys_path(self, tmp_path: Path) -> None:
527+
"""Should restore sys.path after validation."""
528+
agent_file = tmp_path / "agent.py"
529+
agent_file.write_text("root_agent = 'my_agent'\n")
530+
(tmp_path / "__init__.py").touch()
531+
532+
original_path = sys.path.copy()
533+
534+
cli_deploy._validate_agent_import(
535+
str(tmp_path), "root_agent", is_config_agent=False
536+
)
537+
538+
assert sys.path == original_path

0 commit comments

Comments
 (0)