From b1eda859d062a382bfe7e7c252c4bf90797fcdd1 Mon Sep 17 00:00:00 2001 From: Carl Baillargeon Date: Wed, 5 Mar 2025 11:32:04 -0500 Subject: [PATCH] Address review comments --- asynceapi/_models.py | 31 ++++++++++++++------------- asynceapi/_types.py | 4 ++-- asynceapi/device.py | 26 ---------------------- tests/units/asynceapi/test__models.py | 25 ++++++++------------- tests/units/asynceapi/test_device.py | 27 ----------------------- 5 files changed, 27 insertions(+), 86 deletions(-) diff --git a/asynceapi/_models.py b/asynceapi/_models.py index 4f104745a..0572a2f39 100644 --- a/asynceapi/_models.py +++ b/asynceapi/_models.py @@ -6,6 +6,7 @@ from __future__ import annotations from dataclasses import dataclass, field +from logging import getLogger from typing import TYPE_CHECKING, Any, Literal from uuid import uuid4 @@ -17,6 +18,8 @@ from ._types import EapiComplexCommand, EapiJsonOutput, EapiSimpleCommand, EapiTextOutput, JsonRpc +LOGGER = getLogger(__name__) + # pylint: disable=too-many-instance-attributes @dataclass(frozen=True) @@ -102,17 +105,16 @@ def success(self) -> bool: @property def results(self) -> list[EapiCommandResult]: - """Get all results as a list, ordered by command index.""" - return [self._results[i] for i in sorted(self._results.keys())] + """Get all results as a list. Results are ordered by the command indices in the request.""" + return list(self._results.values()) def __len__(self) -> int: """Return the number of results.""" return len(self._results) def __iter__(self) -> Iterator[EapiCommandResult]: - """Enable iteration over the results.""" - for index in sorted(self._results.keys()): - yield self._results[index] + """Enable iteration over the results. Results are yielded in the same order as provided in the request.""" + yield from self._results.values() @classmethod def from_jsonrpc(cls, response: dict[str, Any], request: EapiRequest, *, raise_on_error: bool = False) -> EapiResponse: @@ -120,11 +122,11 @@ def from_jsonrpc(cls, response: dict[str, Any], request: EapiRequest, *, raise_o Parameters ---------- - response : dict + response The JSON-RPC eAPI response dictionary. - request : EapiRequest + request The corresponding EapiRequest. - raise_on_error : bool, optional + raise_on_error Raise an EapiReponseError if the response contains errors, by default False. Returns @@ -162,8 +164,8 @@ def from_jsonrpc(cls, response: dict[str, Any], request: EapiRequest, *, raise_o # Add timestamps if available if request.timestamps and "_meta" in data: meta = data.pop("_meta") - start_time = meta["execStartTime"] - duration = meta["execDuration"] + start_time = meta.get("execStartTime") + duration = meta.get("execDuration") elif isinstance(data, str): # Handle case where eAPI returns a JSON string response (serialized JSON) for certain commands @@ -173,6 +175,7 @@ def from_jsonrpc(cls, response: dict[str, Any], request: EapiRequest, *, raise_o output = loads(data) except (JSONDecodeError, TypeError): # If it's not valid JSON, store as is + LOGGER.warning("Invalid JSON response for command: %s. Storing as text: %s", cmd_str, data) output = data results[i] = EapiCommandResult( @@ -189,9 +192,7 @@ def from_jsonrpc(cls, response: dict[str, Any], request: EapiRequest, *, raise_o for i in range(executed_count, len(request.commands)): cmd = request.commands[i] cmd_str = cmd["cmd"] if isinstance(cmd, dict) else cmd - results[i] = EapiCommandResult( - command=cmd_str, output=None, errors=["Command not executed due to previous error"], success=False, was_executed=False - ) + results[i] = EapiCommandResult(command=cmd_str, output=None, errors=["Command not executed due to previous error"], success=False, executed=False) response_obj = cls( request_id=response["id"], @@ -220,7 +221,7 @@ class EapiCommandResult: A list of error messages, if any. success : bool True if the command was successful. - was_executed : bool + executed : bool True if the command was executed. When `stop_on_error` is True in the request, some commands may not be executed. start_time : float | None Command execution start time in seconds. Uses Unix epoch format. `timestamps` must be True in the request. @@ -232,6 +233,6 @@ class EapiCommandResult: output: EapiJsonOutput | EapiTextOutput | None errors: list[str] = field(default_factory=list) success: bool = True - was_executed: bool = True + executed: bool = True start_time: float | None = None duration: float | None = None diff --git a/asynceapi/_types.py b/asynceapi/_types.py index 6c662bea2..ebebf04b5 100644 --- a/asynceapi/_types.py +++ b/asynceapi/_types.py @@ -21,11 +21,11 @@ EapiTextOutput = str """Type definition of an eAPI text output response.""" EapiSimpleCommand = str -"""Type definition of an eAPI simple command.""" +"""Type definition of an eAPI simple command. A simple command is the CLI command to run as a string.""" class EapiComplexCommand(TypedDict): - """Type definition of an eAPI complex command.""" + """Type definition of an eAPI complex command. A complex command is a dictionary with the CLI command to run with additional parameters.""" cmd: str input: NotRequired[str] diff --git a/asynceapi/device.py b/asynceapi/device.py index 7a7aa933a..a7702daa7 100644 --- a/asynceapi/device.py +++ b/asynceapi/device.py @@ -21,7 +21,6 @@ # Private Imports # ----------------------------------------------------------------------------- from ._constants import EapiCommandFormat -from ._models import EapiRequest, EapiResponse from .aio_portcheck import port_check_url from .config_session import SessionConfig from .errors import EapiCommandError @@ -461,31 +460,6 @@ async def jsonrpc_exec(self, jsonrpc: JsonRpc) -> list[EapiJsonOutput] | list[Ea not_exec=commands[err_at + 1 :], ) - async def _execute(self, request: EapiRequest, *, raise_on_error: bool = False) -> EapiResponse: - """Execute an eAPI request. - - Parameters - ---------- - request - The eAPI request object. - raise_on_error - Raise an EapiReponseError if the eAPI response contains errors. - - Returns - ------- - EapiResponse - The eAPI response object. - - Raises - ------ - EapiReponseError - If the eAPI response contains errors and `raise_on_error` is True. - """ - res = await self.post("/command-api", json=request.to_jsonrpc()) - res.raise_for_status() - body = res.json() - return EapiResponse.from_jsonrpc(body, request, raise_on_error=raise_on_error) - def config_session(self, name: str) -> SessionConfig: """Return a SessionConfig instance bound to this device with the given session name. diff --git a/tests/units/asynceapi/test__models.py b/tests/units/asynceapi/test__models.py index 00622da96..447c24338 100644 --- a/tests/units/asynceapi/test__models.py +++ b/tests/units/asynceapi/test__models.py @@ -3,6 +3,7 @@ # that can be found in the LICENSE file. """Unit tests for the asynceapi._models module.""" +import logging from typing import TYPE_CHECKING from uuid import UUID @@ -174,19 +175,6 @@ def test_len_and_iteration(self) -> None: assert len(iterated_results) == 3 assert [r.command for r in iterated_results] == ["cmd1", "cmd2", "cmd3"] - # Test non-sequential indices - results = { - 5: EapiCommandResult(command="cmd5", output="out5"), - 2: EapiCommandResult(command="cmd2", output="out2"), - 9: EapiCommandResult(command="cmd9", output="out9"), - } - response = EapiResponse(request_id="test-sorted", _results=results) - - # Results should be ordered by index - iterated_results = list(response) - assert [r.command for r in iterated_results] == ["cmd2", "cmd5", "cmd9"] - assert [r.output for r in iterated_results] == ["out2", "out5", "out9"] - def test_from_jsonrpc_success(self) -> None: """Test from_jsonrpc with successful response.""" # Mock request @@ -304,13 +292,13 @@ def test_from_jsonrpc_error_stop_on_error_true(self) -> None: assert response.results[1].output is None assert response.results[1].success is False assert "Command not executed due to previous error" in response.results[1].errors - assert response.results[1].was_executed is False + assert response.results[1].executed is False assert response.results[2].command == "show hostname" assert response.results[2].output is None assert response.results[2].success is False assert "Command not executed due to previous error" in response.results[2].errors - assert response.results[2].was_executed is False + assert response.results[2].executed is False def test_from_jsonrpc_error_stop_on_error_false(self) -> None: """Test from_jsonrpc with error and stop_on_error=False.""" @@ -383,8 +371,10 @@ def test_from_jsonrpc_raise_on_error(self) -> None: assert excinfo.value.response.error_code == 1002 assert excinfo.value.response.error_message == "CLI command 1 of 1 'show bad command' failed: invalid command" - def test_from_jsonrpc_string_data(self) -> None: + def test_from_jsonrpc_string_data(self, caplog: pytest.LogCaptureFixture) -> None: """Test from_jsonrpc with string data response.""" + caplog.set_level(logging.WARNING) + # Mock request request = EapiRequest(commands=["show bgp ipv4 unicast summary", "show bad command"]) @@ -420,6 +410,9 @@ def test_from_jsonrpc_string_data(self) -> None: response = EapiResponse.from_jsonrpc(jsonrpc_response, request) + # Verify WARNING log message + assert "Invalid JSON response for command: show bgp ipv4 unicast summary. Storing as text: This is not JSON" in caplog.text + # Verify string is kept as is assert response.results[0].output == "This is not JSON" diff --git a/tests/units/asynceapi/test_device.py b/tests/units/asynceapi/test_device.py index 0a86e20e3..c15eb3553 100644 --- a/tests/units/asynceapi/test_device.py +++ b/tests/units/asynceapi/test_device.py @@ -11,7 +11,6 @@ from httpx import HTTPStatusError from asynceapi import Device, EapiCommandError -from asynceapi._models import EapiRequest from .test_data import ERROR_EAPI_RESPONSE, JSONRPC_REQUEST_TEMPLATE, SUCCESS_EAPI_RESPONSE @@ -85,29 +84,3 @@ async def test_jsonrpc_exec_http_status_error(asynceapi_device: Device, httpx_mo with pytest.raises(HTTPStatusError): await asynceapi_device.jsonrpc_exec(jsonrpc=jsonrpc_request) - - -async def test__execute(asynceapi_device: Device, httpx_mock: HTTPXMock) -> None: - """Test the Device._execute method.""" - eapi_request = EapiRequest(commands=["show version", "show clock"], id="EapiExplorer-1") - - httpx_mock.add_response(json=SUCCESS_EAPI_RESPONSE) - - eapi_response = await asynceapi_device._execute(eapi_request) - - # Check the response - assert eapi_response.request_id == "EapiExplorer-1" - assert eapi_response.error_code is None - assert eapi_response.error_message is None - assert eapi_response.success is True - - # Check the results - assert len(eapi_response) == 2 - for i, res in enumerate(eapi_response): - assert res.command == eapi_request.commands[i] - assert res.output == SUCCESS_EAPI_RESPONSE["result"][i] - assert res.errors == [] - assert res.success is True - assert res.was_executed is True - assert res.start_time is None - assert res.duration is None