Address review comments

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,29 +105,28 @@ 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:
         """Build an EapiResponse from a JSON-RPC eAPI response.
 
         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

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]

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.
 

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"
 

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