mirror of
https://github.com/encounter/adk-python.git
synced 2026-07-09 18:19:28 -07:00
fix(cli): Handle the case when OS doesn't support symlink for adk run
Merge https://github.com/google/adk-python/pull/2582 Relate to #3306 ## Description Fixes issues #6 and #1785 where ADK commands crash on Windows due to symlink creation requiring admin privileges. ## Problem On Windows, running ADK commands (like `adk run`) fails with `OSError: [WinError 1314] A required privilege is not held by the client: ...` because the logging system attempts to create a symlink for the latest log file. Windows requires administrator privileges for symlink creation by default (unless Developer Mode is enabled), causing crashes for non-admin users. ## Root Cause The issue was in [`logs.py`](https://github.com/google/adk-python/blob/main/src/google/adk/cli/utils/logs.py#L72) where `os.symlink()` was called unconditionally without error handling, causing the entire CLI to crash when symlink creation failed. ## Solution This PR implements graceful symlink handling. Changes made: - Extracted symlink creation into separate function with error handling - Added graceful fallback when symlink creation fails - Replaced crashes with warnings to keep CLI functional - Improved user messaging about log file locations This solution follows a similar pattern to the one used in [ROS2](https://github.com/ros2) in its [logging module](https://github.com/ros2/launch/blob/rolling/launch/launch/logging/__init__.py#L93). ## Testing ### Before (broken) ```bash > adk run my_agent Log setup complete: C:\Users\username\AppData\Local\Temp\agents_log\agent.20250817_215119.log Traceback (most recent call last): File "google\adk\cli\utils\logs.py", line 72, in log_to_tmp_folder os.symlink(log_filepath, latest_log_link) OSError: [WinError 1314] A required privilege is not held by the client ``` ### After (fixed) ```bash > adk run my_agent Log setup complete: C:\Users\username\AppData\Local\Temp\agents_log\agent.20250817_215319.log UserWarning: Cannot create symlink for latest log file: [WinError 1314] A required privilege is not held by the client To access latest log: tail -F C:\Users\username\AppData\Local\Temp\agents_log\agent.20250817_215319.log Running agent my_agent, type exit to exit. ``` Co-authored-by: Wei Sun (Jack) <weisun@google.com> COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/2582 from lorenzofavaro:fix/windows-symlink-permissions 1e99a6287994f1bfd9ac11550d3dc06205998df5 PiperOrigin-RevId: 828690483
This commit is contained in:
committed by
Copybara-Service
parent
1d80d32efc
commit
63b69fbc0f
@@ -18,6 +18,9 @@ import logging
|
||||
import os
|
||||
import tempfile
|
||||
import time
|
||||
import warnings
|
||||
|
||||
import click
|
||||
|
||||
LOGGING_FORMAT = (
|
||||
'%(asctime)s - %(levelname)s - %(filename)s:%(lineno)d - %(message)s'
|
||||
@@ -32,6 +35,38 @@ def setup_adk_logger(level=logging.INFO):
|
||||
adk_logger.setLevel(level)
|
||||
|
||||
|
||||
def _create_symlink(symlink_path: str, target_path: str) -> bool:
|
||||
"""Creates a symlink at symlink_path pointing to target_path.
|
||||
|
||||
Returns:
|
||||
True if successful, False otherwise.
|
||||
"""
|
||||
try:
|
||||
if os.path.islink(symlink_path):
|
||||
os.unlink(symlink_path)
|
||||
elif os.path.exists(symlink_path):
|
||||
warnings.warn(
|
||||
'Cannot create symlink for latest log file: file exists at'
|
||||
f' {symlink_path}'
|
||||
)
|
||||
return False
|
||||
os.symlink(target_path, symlink_path)
|
||||
return True
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def _try_create_latest_log_symlink(
|
||||
log_dir: str, log_file_prefix: str, log_filepath: str
|
||||
) -> None:
|
||||
"""Attempts to create a 'latest' symlink and prints access instructions."""
|
||||
latest_log_link = os.path.join(log_dir, f'{log_file_prefix}.latest.log')
|
||||
if _create_symlink(latest_log_link, log_filepath):
|
||||
click.echo(f'To access latest log: tail -F {latest_log_link}')
|
||||
else:
|
||||
click.echo(f'To access latest log: tail -F {log_filepath}')
|
||||
|
||||
|
||||
def log_to_tmp_folder(
|
||||
level=logging.INFO,
|
||||
*,
|
||||
@@ -64,12 +99,7 @@ def log_to_tmp_folder(
|
||||
root_logger.handlers = [] # Clear handles to disable logging to stderr
|
||||
root_logger.addHandler(file_handler)
|
||||
|
||||
print(f'Log setup complete: {log_filepath}')
|
||||
click.echo(f'Log setup complete: {log_filepath}')
|
||||
_try_create_latest_log_symlink(log_dir, log_file_prefix, log_filepath)
|
||||
|
||||
latest_log_link = os.path.join(log_dir, f'{log_file_prefix}.latest.log')
|
||||
if os.path.islink(latest_log_link):
|
||||
os.unlink(latest_log_link)
|
||||
os.symlink(log_filepath, latest_log_link)
|
||||
|
||||
print(f'To access latest log: tail -F {latest_log_link}')
|
||||
return log_filepath
|
||||
|
||||
Reference in New Issue
Block a user