From 16f54eec46879f72ab6c8c9f69086dd9d40dfaca Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 17:37:28 +0100 Subject: [PATCH 1/9] ci: Add additional ruff lint rules --- .ruff.toml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.ruff.toml b/.ruff.toml index 4c81ecb8..2777447b 100644 --- a/.ruff.toml +++ b/.ruff.toml @@ -29,6 +29,8 @@ ignore = [ "ANN002", "ANN003", "ANN401", + "TRY003", + "G004", ] select = [ @@ -57,6 +59,20 @@ select = [ "TD", # flake8-todos (check TODO format - Google Style ยง3.7) "TCH",# flake8-type-checking (helps manage TYPE_CHECKING blocks and imports) "PYI",# flake8-pyi (best practices for .pyi stub files, some rules are useful for .py too) + "S", # flake8-bandit (security issues) + "DTZ",# flake8-datetimez (timezone-aware datetimes) + "ERA",# flake8-eradicate (commented-out code) + "Q", # flake8-quotes (quote style consistency) + "RSE",# flake8-raise (modern raise statements) + "TRY",# tryceratops (exception handling best practices) + "PERF",# perflint (performance anti-patterns) + "BLE", + "T10", + "ICN", + "G", + "FIX", + "ASYNC", + "INP", ] exclude = [ @@ -104,7 +120,7 @@ ignore-decorators = ["typing.overload", "abc.abstractmethod"] [lint.flake8-annotations] mypy-init-return = true -allow-star-arg-any = true +allow-star-arg-any = false [lint.pep8-naming] ignore-names = ["test_*", "setUp", "tearDown", "mock_*"] From 5b550401023dcb1c1e0c29782c3a26c2679116f6 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 17:47:17 +0100 Subject: [PATCH 2/9] Update formatting script to only check git tracked files --- scripts/format.sh | 112 ++++++++++++++++++++++++---------------------- 1 file changed, 58 insertions(+), 54 deletions(-) diff --git a/scripts/format.sh b/scripts/format.sh index eff5d122..185e79ad 100755 --- a/scripts/format.sh +++ b/scripts/format.sh @@ -8,78 +8,82 @@ FORMAT_ALL=false RUFF_UNSAFE_FIXES_FLAG="" # Process command-line arguments -# We use a while loop with shift to process each argument while [[ "$#" -gt 0 ]]; do - case "$1" in - --all) - FORMAT_ALL=true - echo "Detected --all flag: Formatting all Python files." - shift # Consume the argument - ;; - --unsafe-fixes) - RUFF_UNSAFE_FIXES_FLAG="--unsafe-fixes" - echo "Detected --unsafe-fixes flag: Ruff will run with unsafe fixes." - shift # Consume the argument - ;; - *) - # Handle unknown arguments or just ignore them if we only care about specific ones - echo "Warning: Unknown argument '$1'. Ignoring." - shift # Consume the argument - ;; - esac + case "$1" in + --all) + FORMAT_ALL=true + echo "Detected --all flag: Formatting all tracked Python files." + shift # Consume the argument + ;; + --unsafe-fixes) + RUFF_UNSAFE_FIXES_FLAG="--unsafe-fixes" + echo "Detected --unsafe-fixes flag: Ruff will run with unsafe fixes." + shift # Consume the argument + ;; + *) + # Handle unknown arguments or just ignore them + echo "Warning: Unknown argument '$1'. Ignoring." + shift # Consume the argument + ;; + esac done # Sort Spelling Allowlist SPELLING_ALLOW_FILE=".github/actions/spelling/allow.txt" if [ -f "$SPELLING_ALLOW_FILE" ]; then - echo "Sorting and de-duplicating $SPELLING_ALLOW_FILE" - sort -u "$SPELLING_ALLOW_FILE" -o "$SPELLING_ALLOW_FILE" + echo "Sorting and de-duplicating $SPELLING_ALLOW_FILE" + sort -u "$SPELLING_ALLOW_FILE" -o "$SPELLING_ALLOW_FILE" fi CHANGED_FILES="" if $FORMAT_ALL; then - echo "Formatting all Python files in the repository." - # Find all Python files, excluding grpc generated files as per original logic. - # `sort -u` ensures unique files and consistent ordering for display/xargs. - CHANGED_FILES=$(find . -name '*.py' -not -path './src/a2a/grpc/*' | sort -u) - - if [ -z "$CHANGED_FILES" ]; then - echo "No Python files found to format." - exit 0 - fi + echo "Finding all tracked Python files in the repository..." + CHANGED_FILES=$(git ls-files -- '*.py' ':!src/a2a/grpc/*') else - echo "No '--all' flag found. Formatting changed Python files based on git diff." - TARGET_BRANCH="origin/${GITHUB_BASE_REF:-main}" - git fetch origin "${GITHUB_BASE_REF:-main}" --depth=1 - - MERGE_BASE=$(git merge-base HEAD "$TARGET_BRANCH") + echo "Finding changed Python files based on git diff..." + TARGET_BRANCH="origin/${GITHUB_BASE_REF:-main}" + git fetch origin "${GITHUB_BASE_REF:-main}" --depth=1 - # Get python files changed in this PR, excluding grpc generated files - CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB "$MERGE_BASE" HEAD -- '*.py' ':!src/a2a/grpc/*') + MERGE_BASE=$(git merge-base HEAD "$TARGET_BRANCH") - if [ -z "$CHANGED_FILES" ]; then - echo "No changed Python files to format." - exit 0 - fi + # Get python files changed in this PR, excluding grpc generated files. + CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB "$MERGE_BASE" HEAD -- '*.py' ':!src/a2a/grpc/*') fi -echo "Files to be formatted:" -echo "$CHANGED_FILES" +# Exit if no files were found +if [ -z "$CHANGED_FILES" ]; then + echo "No changed or tracked Python files to format." + exit 0 +fi -# Helper function to run formatters with the list of files. -# The list of files is passed to xargs via stdin. +# --- Helper Function --- +# Runs a command on a list of files passed via stdin. +# $1: A string containing the list of files (space-separated). +# $2...: The command and its arguments to run. run_formatter() { - echo "$CHANGED_FILES" | xargs -r "$@" + local files_to_format="$1" + shift # Remove the file list from the arguments + if [ -n "$files_to_format" ]; then + echo "$files_to_format" | xargs -r "$@" + fi } -echo "Running pyupgrade..." -run_formatter pyupgrade --exit-zero-even-if-changed --py310-plus -echo "Running autoflake..." -run_formatter autoflake -i -r --remove-all-unused-imports -echo "Running ruff check (fix-only)..." -run_formatter ruff check --fix $RUFF_UNSAFE_FIXES_FLAG -echo "Running ruff format..." -run_formatter ruff format +# --- Python File Formatting --- +if [ -n "$CHANGED_FILES" ]; then + echo "--- Formatting Python Files ---" + echo "Files to be formatted:" + echo "$CHANGED_FILES" + + echo "Running autoflake..." + run_formatter "$CHANGED_FILES" autoflake -i -r --remove-all-unused-imports + echo "Running ruff check (fix-only)..." + run_formatter "$CHANGED_FILES" ruff check --fix-only $RUFF_UNSAFE_FIXES_FLAG + echo "Running ruff format..." + run_formatter "$CHANGED_FILES" ruff format + echo "Python formatting complete." +else + echo "No Python files to format." +fi -echo "Formatting complete." +echo "All formatting tasks are complete." From 773dbd8eee0cdff826f4f45e00085dc0c8482979 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 17:53:37 +0100 Subject: [PATCH 3/9] Add exceptions --- .ruff.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ruff.toml b/.ruff.toml index 2777447b..606e32dc 100644 --- a/.ruff.toml +++ b/.ruff.toml @@ -31,6 +31,7 @@ ignore = [ "ANN401", "TRY003", "G004", + "TRY201", ] select = [ @@ -155,6 +156,7 @@ inline-quotes = "single" "types.py" = ["D", "E501"] # Ignore docstring and annotation issues in types.py "proto_utils.py" = ["D102", "PLR0911"] "helpers.py" = ["ANN001", "ANN201", "ANN202"] +"scripts/*.py" = ["INP001"] [format] exclude = [ From 78add2ebc6d4cf0f12b6361ec4ff3810d0eec047 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 17:55:56 +0100 Subject: [PATCH 4/9] Run ruff format --- src/a2a/server/apps/jsonrpc/jsonrpc_app.py | 2 +- src/a2a/server/events/event_consumer.py | 4 ++-- src/a2a/server/events/in_memory_queue_manager.py | 4 ++-- .../request_handlers/default_request_handler.py | 2 +- .../tasks/base_push_notification_sender.py | 2 +- .../database_push_notification_config_store.py | 8 ++++---- src/a2a/utils/telemetry.py | 4 ++-- tests/client/test_base_client.py | 5 ++--- tests/client/test_client_task_manager.py | 16 +++++++++------- tests/server/apps/jsonrpc/test_serialization.py | 2 +- tests/server/apps/rest/test_rest_fastapi_app.py | 6 ++---- 11 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/a2a/server/apps/jsonrpc/jsonrpc_app.py b/src/a2a/server/apps/jsonrpc/jsonrpc_app.py index 5c841feb..d750ad9c 100644 --- a/src/a2a/server/apps/jsonrpc/jsonrpc_app.py +++ b/src/a2a/server/apps/jsonrpc/jsonrpc_app.py @@ -317,7 +317,7 @@ async def _handle_requests(self, request: Request) -> Response: # noqa: PLR0911 ) raise e except Exception as e: - logger.error(f'Unhandled exception: {e}') + logger.exception(f'Unhandled exception: {e}') traceback.print_exc() return self._generate_error_response( request_id, A2AError(root=InternalError(message=str(e))) diff --git a/src/a2a/server/events/event_consumer.py b/src/a2a/server/events/event_consumer.py index 607c3c9c..a5a00e13 100644 --- a/src/a2a/server/events/event_consumer.py +++ b/src/a2a/server/events/event_consumer.py @@ -141,10 +141,10 @@ async def consume_all(self) -> AsyncGenerator[Event]: if self.queue.is_closed(): break except ValidationError as e: - logger.error(f'Invalid event format received: {e}') + logger.exception(f'Invalid event format received: {e}') continue except Exception as e: - logger.error( + logger.exception( f'Stopping event consumption due to exception: {e}' ) self._exception = e diff --git a/src/a2a/server/events/in_memory_queue_manager.py b/src/a2a/server/events/in_memory_queue_manager.py index 6a2da4b9..53a3b7dd 100644 --- a/src/a2a/server/events/in_memory_queue_manager.py +++ b/src/a2a/server/events/in_memory_queue_manager.py @@ -34,7 +34,7 @@ async def add(self, task_id: str, queue: EventQueue) -> None: """ async with self._lock: if task_id in self._task_queue: - raise TaskQueueExists() + raise TaskQueueExists self._task_queue[task_id] = queue async def get(self, task_id: str) -> EventQueue | None: @@ -67,7 +67,7 @@ async def close(self, task_id: str) -> None: """ async with self._lock: if task_id not in self._task_queue: - raise NoTaskQueue() + raise NoTaskQueue queue = self._task_queue.pop(task_id) await queue.close() diff --git a/src/a2a/server/request_handlers/default_request_handler.py b/src/a2a/server/request_handlers/default_request_handler.py index 1dfce760..1204e80a 100644 --- a/src/a2a/server/request_handlers/default_request_handler.py +++ b/src/a2a/server/request_handlers/default_request_handler.py @@ -303,7 +303,7 @@ async def on_message_send( ) except Exception as e: - logger.error(f'Agent execution failed. Error: {e}') + logger.exception(f'Agent execution failed. Error: {e}') raise finally: if interrupted_or_non_blocking: diff --git a/src/a2a/server/tasks/base_push_notification_sender.py b/src/a2a/server/tasks/base_push_notification_sender.py index 51558d42..7e0cda77 100644 --- a/src/a2a/server/tasks/base_push_notification_sender.py +++ b/src/a2a/server/tasks/base_push_notification_sender.py @@ -66,7 +66,7 @@ async def _dispatch_notification( ) return True except Exception as e: - logger.error( + logger.exception( f'Error sending push-notification for task_id={task.id} to URL: {url}. Error: {e}' ) return False diff --git a/src/a2a/server/tasks/database_push_notification_config_store.py b/src/a2a/server/tasks/database_push_notification_config_store.py index dbe1e4ff..645b4639 100644 --- a/src/a2a/server/tasks/database_push_notification_config_store.py +++ b/src/a2a/server/tasks/database_push_notification_config_store.py @@ -175,7 +175,7 @@ def _from_orm( decrypted_payload ) except (json.JSONDecodeError, ValidationError) as e: - logger.error( + logger.exception( 'Failed to parse decrypted push notification config for task %s, config %s. ' 'Data is corrupted or not valid JSON after decryption.', model_instance.task_id, @@ -201,7 +201,7 @@ def _from_orm( return PushNotificationConfig.model_validate_json(payload) except (json.JSONDecodeError, ValidationError) as e: if self._fernet: - logger.error( + logger.exception( 'Failed to parse push notification config for task %s, config %s. ' 'Decryption failed and the data is not valid JSON. ' 'This likely indicates the data is corrupted or encrypted with a different key.', @@ -210,7 +210,7 @@ def _from_orm( ) else: # if no key is configured and the payload is not valid JSON. - logger.error( + logger.exception( 'Failed to parse push notification config for task %s, config %s. ' 'Data is not valid JSON and no encryption key is configured.', model_instance.task_id, @@ -253,7 +253,7 @@ async def get_info(self, task_id: str) -> list[PushNotificationConfig]: try: configs.append(self._from_orm(model)) except ValueError as e: - logger.error( + logger.exception( 'Could not deserialize push notification config for task %s, config %s: %s', model.task_id, model.config_id, diff --git a/src/a2a/utils/telemetry.py b/src/a2a/utils/telemetry.py index f911fd6b..7a6fb741 100644 --- a/src/a2a/utils/telemetry.py +++ b/src/a2a/utils/telemetry.py @@ -213,7 +213,7 @@ async def async_wrapper(*args, **kwargs) -> Any: span, args, kwargs, result, exception ) except Exception as attr_e: - logger.error( + logger.exception( f'attribute_extractor error in span {actual_span_name}: {attr_e}' ) @@ -247,7 +247,7 @@ def sync_wrapper(*args, **kwargs) -> Any: span, args, kwargs, result, exception ) except Exception as attr_e: - logger.error( + logger.exception( f'attribute_extractor error in span {actual_span_name}: {attr_e}' ) diff --git a/tests/client/test_base_client.py b/tests/client/test_base_client.py index 7b1aacec..c1251f1c 100644 --- a/tests/client/test_base_client.py +++ b/tests/client/test_base_client.py @@ -1,4 +1,4 @@ -from unittest.mock import AsyncMock, MagicMock, patch +from unittest.mock import AsyncMock, MagicMock import pytest @@ -20,8 +20,7 @@ @pytest.fixture def mock_transport(): - transport = AsyncMock(spec=ClientTransport) - return transport + return AsyncMock(spec=ClientTransport) @pytest.fixture diff --git a/tests/client/test_client_task_manager.py b/tests/client/test_client_task_manager.py index fd626d2c..b07ddceb 100644 --- a/tests/client/test_client_task_manager.py +++ b/tests/client/test_client_task_manager.py @@ -1,21 +1,23 @@ -import pytest from unittest.mock import AsyncMock, Mock, patch + +import pytest + from a2a.client.client_task_manager import ClientTaskManager from a2a.client.errors import ( A2AClientInvalidArgsError, A2AClientInvalidStateError, ) from a2a.types import ( + Artifact, + Message, + Part, + Role, Task, - TaskStatus, + TaskArtifactUpdateEvent, TaskState, + TaskStatus, TaskStatusUpdateEvent, - TaskArtifactUpdateEvent, - Message, - Role, - Part, TextPart, - Artifact, ) diff --git a/tests/server/apps/jsonrpc/test_serialization.py b/tests/server/apps/jsonrpc/test_serialization.py index 1670bb96..df2e8a3a 100644 --- a/tests/server/apps/jsonrpc/test_serialization.py +++ b/tests/server/apps/jsonrpc/test_serialization.py @@ -1,8 +1,8 @@ from unittest import mock import pytest -from fastapi import FastAPI +from fastapi import FastAPI from pydantic import ValidationError from starlette.testclient import TestClient diff --git a/tests/server/apps/rest/test_rest_fastapi_app.py b/tests/server/apps/rest/test_rest_fastapi_app.py index ff463880..c5ea89c4 100644 --- a/tests/server/apps/rest/test_rest_fastapi_app.py +++ b/tests/server/apps/rest/test_rest_fastapi_app.py @@ -1,7 +1,6 @@ import logging from typing import Any - from unittest.mock import MagicMock import pytest @@ -11,10 +10,9 @@ from httpx import ASGITransport, AsyncClient from a2a.grpc import a2a_pb2 -from a2a.server.apps.rest import fastapi_app -from a2a.server.apps.rest import rest_adapter -from a2a.server.apps.rest.rest_adapter import RESTAdapter +from a2a.server.apps.rest import fastapi_app, rest_adapter from a2a.server.apps.rest.fastapi_app import A2ARESTFastAPIApplication +from a2a.server.apps.rest.rest_adapter import RESTAdapter from a2a.server.request_handlers.request_handler import RequestHandler from a2a.types import ( AgentCard, From 29f104156b84c5948678106fc6674ddb389abed2 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 17:56:43 +0100 Subject: [PATCH 5/9] Add lint ignore --- tests/server/apps/jsonrpc/test_jsonrpc_app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/server/apps/jsonrpc/test_jsonrpc_app.py b/tests/server/apps/jsonrpc/test_jsonrpc_app.py index 6b86e7e7..72da7377 100644 --- a/tests/server/apps/jsonrpc/test_jsonrpc_app.py +++ b/tests/server/apps/jsonrpc/test_jsonrpc_app.py @@ -112,8 +112,8 @@ class TestJSONRPCApplicationOptionalDeps: @pytest.fixture(scope='class', autouse=True) def ensure_pkg_starlette_is_present(self): try: - import starlette as _starlette - import sse_starlette as _sse_starlette + import sse_starlette as _sse_starlette # noqa: F401 + import starlette as _starlette # noqa: F401 except ImportError: pytest.fail( f'Running tests in {self.__class__.__name__} requires' From f8ae67d2d60dec65272100339eabb53d1aa88c64 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 18:23:14 +0100 Subject: [PATCH 6/9] Add lint fixes --- scripts/grpc_gen_post_processor.py | 2 +- src/a2a/extensions/__init__.py | 0 src/a2a/server/apps/jsonrpc/jsonrpc_app.py | 3 +-- src/a2a/server/events/event_consumer.py | 8 +++---- src/a2a/server/events/event_queue.py | 5 +++-- .../default_request_handler.py | 20 +++++++----------- .../tasks/base_push_notification_sender.py | 6 +++--- ...database_push_notification_config_store.py | 5 ++--- src/a2a/utils/error_handlers.py | 2 +- src/a2a/utils/proto_utils.py | 21 +++++++------------ src/a2a/utils/telemetry.py | 13 ++++++------ 11 files changed, 36 insertions(+), 49 deletions(-) create mode 100644 src/a2a/extensions/__init__.py diff --git a/scripts/grpc_gen_post_processor.py b/scripts/grpc_gen_post_processor.py index ce6b3612..10a02caf 100644 --- a/scripts/grpc_gen_post_processor.py +++ b/scripts/grpc_gen_post_processor.py @@ -47,7 +47,7 @@ def process_generated_code(src_folder: str = 'src/a2a/grpc') -> None: else: print('No changes needed') - except Exception as e: + except Exception as e: # noqa: BLE001 print(f'Error processing file {file}: {e}') sys.exit(1) diff --git a/src/a2a/extensions/__init__.py b/src/a2a/extensions/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/a2a/server/apps/jsonrpc/jsonrpc_app.py b/src/a2a/server/apps/jsonrpc/jsonrpc_app.py index d750ad9c..46e79a2c 100644 --- a/src/a2a/server/apps/jsonrpc/jsonrpc_app.py +++ b/src/a2a/server/apps/jsonrpc/jsonrpc_app.py @@ -317,8 +317,7 @@ async def _handle_requests(self, request: Request) -> Response: # noqa: PLR0911 ) raise e except Exception as e: - logger.exception(f'Unhandled exception: {e}') - traceback.print_exc() + logger.exception('Unhandled exception') return self._generate_error_response( request_id, A2AError(root=InternalError(message=str(e))) ) diff --git a/src/a2a/server/events/event_consumer.py b/src/a2a/server/events/event_consumer.py index a5a00e13..23ab9487 100644 --- a/src/a2a/server/events/event_consumer.py +++ b/src/a2a/server/events/event_consumer.py @@ -140,13 +140,11 @@ async def consume_all(self) -> AsyncGenerator[Event]: # python 3.12 and get a queue empty error on an open queue if self.queue.is_closed(): break - except ValidationError as e: - logger.exception(f'Invalid event format received: {e}') + except ValidationError: + logger.exception('Invalid event format received') continue except Exception as e: - logger.exception( - f'Stopping event consumption due to exception: {e}' - ) + logger.exception('Stopping event consumption due to exception') self._exception = e continue diff --git a/src/a2a/server/events/event_queue.py b/src/a2a/server/events/event_queue.py index 1ce2bd21..6bf9650c 100644 --- a/src/a2a/server/events/event_queue.py +++ b/src/a2a/server/events/event_queue.py @@ -147,8 +147,9 @@ async def close(self) -> None: # Otherwise, join the queue else: tasks = [asyncio.create_task(self.queue.join())] - for child in self._children: - tasks.append(asyncio.create_task(child.close())) + tasks.extend( + asyncio.create_task(child.close()) for child in self._children + ) await asyncio.wait(tasks, return_when=asyncio.ALL_COMPLETED) def is_closed(self) -> bool: diff --git a/src/a2a/server/request_handlers/default_request_handler.py b/src/a2a/server/request_handlers/default_request_handler.py index 1204e80a..3c0746d2 100644 --- a/src/a2a/server/request_handlers/default_request_handler.py +++ b/src/a2a/server/request_handlers/default_request_handler.py @@ -302,8 +302,8 @@ async def on_message_send( task_id, result_aggregator ) - except Exception as e: - logger.exception(f'Agent execution failed. Error: {e}') + except Exception: + logger.exception('Agent execution failed') raise finally: if interrupted_or_non_blocking: @@ -478,16 +478,12 @@ async def on_list_task_push_notification_config( params.id ) - task_push_notification_config = [] - if push_notification_config_list: - for config in push_notification_config_list: - task_push_notification_config.append( - TaskPushNotificationConfig( - task_id=params.id, push_notification_config=config - ) - ) - - return task_push_notification_config + return [ + TaskPushNotificationConfig( + task_id=params.id, push_notification_config=config + ) + for config in push_notification_config_list + ] async def on_delete_task_push_notification_config( self, diff --git a/src/a2a/server/tasks/base_push_notification_sender.py b/src/a2a/server/tasks/base_push_notification_sender.py index 7e0cda77..22139ee7 100644 --- a/src/a2a/server/tasks/base_push_notification_sender.py +++ b/src/a2a/server/tasks/base_push_notification_sender.py @@ -64,9 +64,9 @@ async def _dispatch_notification( logger.info( f'Push-notification sent for task_id={task.id} to URL: {url}' ) - return True - except Exception as e: + except Exception: logger.exception( - f'Error sending push-notification for task_id={task.id} to URL: {url}. Error: {e}' + f'Error sending push-notification for task_id={task.id} to URL: {url}.' ) return False + return True diff --git a/src/a2a/server/tasks/database_push_notification_config_store.py b/src/a2a/server/tasks/database_push_notification_config_store.py index 645b4639..fb541498 100644 --- a/src/a2a/server/tasks/database_push_notification_config_store.py +++ b/src/a2a/server/tasks/database_push_notification_config_store.py @@ -252,12 +252,11 @@ async def get_info(self, task_id: str) -> list[PushNotificationConfig]: for model in models: try: configs.append(self._from_orm(model)) - except ValueError as e: + except ValueError: logger.exception( - 'Could not deserialize push notification config for task %s, config %s: %s', + 'Could not deserialize push notification config for task %s, config %s', model.task_id, model.config_id, - e, ) return configs diff --git a/src/a2a/utils/error_handlers.py b/src/a2a/utils/error_handlers.py index 6955ec60..a9629ce4 100644 --- a/src/a2a/utils/error_handlers.py +++ b/src/a2a/utils/error_handlers.py @@ -80,7 +80,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> Response: return JSONResponse( content={'message': error.message}, status_code=http_code ) - except Exception as e: + except Exception as e: # noqa: BLE001 logger.log(logging.ERROR, f'Unknown error occurred {e}') return JSONResponse( content={'message': 'unknown exception'}, status_code=500 diff --git a/src/a2a/utils/proto_utils.py b/src/a2a/utils/proto_utils.py index e67f0dfa..3f3db578 100644 --- a/src/a2a/utils/proto_utils.py +++ b/src/a2a/utils/proto_utils.py @@ -338,16 +338,12 @@ def security( ) -> list[a2a_pb2.Security] | None: if not security: return None - rval: list[a2a_pb2.Security] = [] - for s in security: - rval.append( - a2a_pb2.Security( - schemes={ - k: a2a_pb2.StringList(list=v) for (k, v) in s.items() - } - ) + return [ + a2a_pb2.Security( + schemes={k: a2a_pb2.StringList(list=v) for (k, v) in s.items()} ) - return rval + for s in security + ] @classmethod def security_schemes( @@ -774,10 +770,9 @@ def security( ) -> list[dict[str, list[str]]] | None: if not security: return None - rval: list[dict[str, list[str]]] = [] - for s in security: - rval.append({k: list(v.list) for (k, v) in s.schemes.items()}) - return rval + return [ + {k: list(v.list) for (k, v) in s.schemes.items()} for s in security + ] @classmethod def provider( diff --git a/src/a2a/utils/telemetry.py b/src/a2a/utils/telemetry.py index 7a6fb741..298b86e4 100644 --- a/src/a2a/utils/telemetry.py +++ b/src/a2a/utils/telemetry.py @@ -193,8 +193,6 @@ async def async_wrapper(*args, **kwargs) -> Any: # Async wrapper, await for the function call to complete. result = await func(*args, **kwargs) span.set_status(StatusCode.OK) - return result - # asyncio.CancelledError extends from BaseException except asyncio.CancelledError as ce: exception = None @@ -212,10 +210,11 @@ async def async_wrapper(*args, **kwargs) -> Any: attribute_extractor( span, args, kwargs, result, exception ) - except Exception as attr_e: + except Exception: logger.exception( - f'attribute_extractor error in span {actual_span_name}: {attr_e}' + f'attribute_extractor error in span {actual_span_name}' ) + return result @functools.wraps(func) def sync_wrapper(*args, **kwargs) -> Any: @@ -233,7 +232,6 @@ def sync_wrapper(*args, **kwargs) -> Any: # Sync wrapper, execute the function call. result = func(*args, **kwargs) span.set_status(StatusCode.OK) - return result except Exception as e: exception = e @@ -246,10 +244,11 @@ def sync_wrapper(*args, **kwargs) -> Any: attribute_extractor( span, args, kwargs, result, exception ) - except Exception as attr_e: + except Exception: logger.exception( - f'attribute_extractor error in span {actual_span_name}: {attr_e}' + f'attribute_extractor error in span {actual_span_name}' ) + return result return async_wrapper if is_async_func else sync_wrapper From 98a5083639a9bc3eb2a653c6aac80feb74bd6f5d Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 19:15:37 +0100 Subject: [PATCH 7/9] Convert to spaces --- scripts/format.sh | 90 +++++++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/scripts/format.sh b/scripts/format.sh index 185e79ad..2be6be3d 100755 --- a/scripts/format.sh +++ b/scripts/format.sh @@ -9,52 +9,52 @@ RUFF_UNSAFE_FIXES_FLAG="" # Process command-line arguments while [[ "$#" -gt 0 ]]; do - case "$1" in - --all) - FORMAT_ALL=true - echo "Detected --all flag: Formatting all tracked Python files." - shift # Consume the argument - ;; - --unsafe-fixes) - RUFF_UNSAFE_FIXES_FLAG="--unsafe-fixes" - echo "Detected --unsafe-fixes flag: Ruff will run with unsafe fixes." - shift # Consume the argument - ;; - *) - # Handle unknown arguments or just ignore them - echo "Warning: Unknown argument '$1'. Ignoring." - shift # Consume the argument - ;; - esac + case "$1" in + --all) + FORMAT_ALL=true + echo "Detected --all flag: Formatting all tracked Python files." + shift # Consume the argument + ;; + --unsafe-fixes) + RUFF_UNSAFE_FIXES_FLAG="--unsafe-fixes" + echo "Detected --unsafe-fixes flag: Ruff will run with unsafe fixes." + shift # Consume the argument + ;; + *) + # Handle unknown arguments or just ignore them + echo "Warning: Unknown argument '$1'. Ignoring." + shift # Consume the argument + ;; + esac done # Sort Spelling Allowlist SPELLING_ALLOW_FILE=".github/actions/spelling/allow.txt" if [ -f "$SPELLING_ALLOW_FILE" ]; then - echo "Sorting and de-duplicating $SPELLING_ALLOW_FILE" - sort -u "$SPELLING_ALLOW_FILE" -o "$SPELLING_ALLOW_FILE" + echo "Sorting and de-duplicating $SPELLING_ALLOW_FILE" + sort -u "$SPELLING_ALLOW_FILE" -o "$SPELLING_ALLOW_FILE" fi CHANGED_FILES="" if $FORMAT_ALL; then - echo "Finding all tracked Python files in the repository..." - CHANGED_FILES=$(git ls-files -- '*.py' ':!src/a2a/grpc/*') + echo "Finding all tracked Python files in the repository..." + CHANGED_FILES=$(git ls-files -- '*.py' ':!src/a2a/grpc/*') else - echo "Finding changed Python files based on git diff..." - TARGET_BRANCH="origin/${GITHUB_BASE_REF:-main}" - git fetch origin "${GITHUB_BASE_REF:-main}" --depth=1 + echo "Finding changed Python files based on git diff..." + TARGET_BRANCH="origin/${GITHUB_BASE_REF:-main}" + git fetch origin "${GITHUB_BASE_REF:-main}" --depth=1 - MERGE_BASE=$(git merge-base HEAD "$TARGET_BRANCH") + MERGE_BASE=$(git merge-base HEAD "$TARGET_BRANCH") - # Get python files changed in this PR, excluding grpc generated files. - CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB "$MERGE_BASE" HEAD -- '*.py' ':!src/a2a/grpc/*') + # Get python files changed in this PR, excluding grpc generated files. + CHANGED_FILES=$(git diff --name-only --diff-filter=ACMRTUXB "$MERGE_BASE" HEAD -- '*.py' ':!src/a2a/grpc/*') fi # Exit if no files were found if [ -z "$CHANGED_FILES" ]; then - echo "No changed or tracked Python files to format." - exit 0 + echo "No changed or tracked Python files to format." + exit 0 fi # --- Helper Function --- @@ -62,28 +62,28 @@ fi # $1: A string containing the list of files (space-separated). # $2...: The command and its arguments to run. run_formatter() { - local files_to_format="$1" - shift # Remove the file list from the arguments - if [ -n "$files_to_format" ]; then - echo "$files_to_format" | xargs -r "$@" - fi + local files_to_format="$1" + shift # Remove the file list from the arguments + if [ -n "$files_to_format" ]; then + echo "$files_to_format" | xargs -r "$@" + fi } # --- Python File Formatting --- if [ -n "$CHANGED_FILES" ]; then - echo "--- Formatting Python Files ---" - echo "Files to be formatted:" - echo "$CHANGED_FILES" + echo "--- Formatting Python Files ---" + echo "Files to be formatted:" + echo "$CHANGED_FILES" - echo "Running autoflake..." - run_formatter "$CHANGED_FILES" autoflake -i -r --remove-all-unused-imports - echo "Running ruff check (fix-only)..." - run_formatter "$CHANGED_FILES" ruff check --fix-only $RUFF_UNSAFE_FIXES_FLAG - echo "Running ruff format..." - run_formatter "$CHANGED_FILES" ruff format - echo "Python formatting complete." + echo "Running autoflake..." + run_formatter "$CHANGED_FILES" autoflake -i -r --remove-all-unused-imports + echo "Running ruff check (fix-only)..." + run_formatter "$CHANGED_FILES" ruff check --fix-only $RUFF_UNSAFE_FIXES_FLAG + echo "Running ruff format..." + run_formatter "$CHANGED_FILES" ruff format + echo "Python formatting complete." else - echo "No Python files to format." + echo "No Python files to format." fi echo "All formatting tasks are complete." From c1d59c3054abef69ada7b613b61917e448853ac1 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 19:26:05 +0100 Subject: [PATCH 8/9] Adjust tests to check for `logger.exception` --- .../tasks/test_inmemory_push_notifications.py | 14 ++++++++------ .../server/tasks/test_push_notification_sender.py | 2 +- tests/utils/test_telemetry.py | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/server/tasks/test_inmemory_push_notifications.py b/tests/server/tasks/test_inmemory_push_notifications.py index aef850c9..6034e7f2 100644 --- a/tests/server/tasks/test_inmemory_push_notifications.py +++ b/tests/server/tasks/test_inmemory_push_notifications.py @@ -220,12 +220,13 @@ async def test_send_notification_http_status_error( await self.notifier.send_notification(task_data) # Pass only task_data self.mock_httpx_client.post.assert_awaited_once() - mock_logger.error.assert_called_once() + mock_logger.exception.assert_called_once() # Check that the error message contains the generic part and the specific exception string self.assertIn( - 'Error sending push-notification', mock_logger.error.call_args[0][0] + 'Error sending push-notification', + mock_logger.exception.call_args[0][0], ) - self.assertIn(str(http_error), mock_logger.error.call_args[0][0]) + self.assertIn(str(http_error), mock_logger.exception.call_args[0][0]) @patch('a2a.server.tasks.base_push_notification_sender.logger') async def test_send_notification_request_error( @@ -242,11 +243,12 @@ async def test_send_notification_request_error( await self.notifier.send_notification(task_data) # Pass only task_data self.mock_httpx_client.post.assert_awaited_once() - mock_logger.error.assert_called_once() + mock_logger.exception.assert_called_once() self.assertIn( - 'Error sending push-notification', mock_logger.error.call_args[0][0] + 'Error sending push-notification', + mock_logger.exception.call_args[0][0], ) - self.assertIn(str(request_error), mock_logger.error.call_args[0][0]) + self.assertIn(str(request_error), mock_logger.exception.call_args[0][0]) @patch('a2a.server.tasks.base_push_notification_sender.logger') async def test_send_notification_with_auth(self, mock_logger: MagicMock): diff --git a/tests/server/tasks/test_push_notification_sender.py b/tests/server/tasks/test_push_notification_sender.py index 6cd4521d..fb398670 100644 --- a/tests/server/tasks/test_push_notification_sender.py +++ b/tests/server/tasks/test_push_notification_sender.py @@ -123,7 +123,7 @@ async def test_send_notification_http_status_error( json=task_data.model_dump(mode='json', exclude_none=True), headers=None, ) - mock_logger.error.assert_called_once() + mock_logger.exception.assert_called_once() async def test_send_notification_multiple_configs(self): task_id = 'task_multiple_configs' diff --git a/tests/utils/test_telemetry.py b/tests/utils/test_telemetry.py index 1a1040a0..62955ccb 100644 --- a/tests/utils/test_telemetry.py +++ b/tests/utils/test_telemetry.py @@ -78,7 +78,7 @@ def foo() -> int: return 1 foo() - logger.error.assert_any_call(mock.ANY) + logger.exception.assert_any_call(mock.ANY) @pytest.mark.asyncio From 2324f8e0c086890a84db2c19ea14c1d9b73b9939 Mon Sep 17 00:00:00 2001 From: Holt Skinner Date: Tue, 12 Aug 2025 19:34:39 +0100 Subject: [PATCH 9/9] Remove redundant checks in tests --- tests/server/tasks/test_inmemory_push_notifications.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/server/tasks/test_inmemory_push_notifications.py b/tests/server/tasks/test_inmemory_push_notifications.py index 6034e7f2..93baf0d3 100644 --- a/tests/server/tasks/test_inmemory_push_notifications.py +++ b/tests/server/tasks/test_inmemory_push_notifications.py @@ -226,7 +226,6 @@ async def test_send_notification_http_status_error( 'Error sending push-notification', mock_logger.exception.call_args[0][0], ) - self.assertIn(str(http_error), mock_logger.exception.call_args[0][0]) @patch('a2a.server.tasks.base_push_notification_sender.logger') async def test_send_notification_request_error( @@ -248,7 +247,6 @@ async def test_send_notification_request_error( 'Error sending push-notification', mock_logger.exception.call_args[0][0], ) - self.assertIn(str(request_error), mock_logger.exception.call_args[0][0]) @patch('a2a.server.tasks.base_push_notification_sender.logger') async def test_send_notification_with_auth(self, mock_logger: MagicMock):