Hi Dean, it looks mostly good, just some nits.
On 21/01/2025 20:41, Dean Marx wrote:
+ def flow_create(self, flow_rule: FlowRule, port_id: int, verify: bool = True)
-> int:
+ """Creates a flow rule in the testpmd session.
+
+ Args:
+ flow_rule: :class:`FlowRule` object used for creating testpmd flow
rule.
+ port_id: Integer representing the port to use.
+ verify: If :data:`True`, the output of the command is scanned
+ to ensure the flow rule was created successfully.
This line should be indented further.
+
+ Raises:
+ InteractiveCommandExecutionError: If flow rule is invalid.
+
+ Returns:
+ Id of created flow rule as an integer.
There is no reason to specify the type when it's already annotated as
part of the function signature.
+ """
+ flow_output = self.send_command(f"flow create {port_id} {flow_rule}")
+ if verify:
+ if "created" not in flow_output:
+ self._logger.debug(f"Failed to create flow
rule:\n{flow_output}")
+ raise InteractiveCommandExecutionError(
+ f"Failed to create flow rule:\n{flow_output}"
+ )
With the check below here, we are already verifying the command
execution... as you are effectively testing the same output. Therefore
this verification above is redundant. I'd remove it, together with the
verify argument. Finally, I'd specify in the description of the
docstring that this function by returning the number of the created flow
it's implicitly verifying its execution.
+ match = re.search(r"#(\d+)", flow_output)
+ if match is not None:
+ match_str = match.group(1)
+ flow_id = int(match_str)
+ return flow_id
+ else:
+ self._logger.debug(f"Failed to create flow rule:\n{flow_output}")
+ raise InteractiveCommandExecutionError(f"Failed to create flow
rule:\n{flow_output}")
+
+ def flow_delete(self, flow_id: int, port_id: int, verify: bool = True) ->
None:
+ """Deletes the specified flow rule from the testpmd session.
+
+ Args:
+ flow_id: :class:`FlowRule` id used for deleting testpmd flow rule.
I guess it's not really a FlowRule id. Just Flow id. So:
Id of the flow to remove.
+ port_id: Integer representing the port to use.
+ verify: If :data:`True`, the output of the command is scanned
+ to ensure the flow rule was deleted successfully.
indent
