On 10/04/2024 08:41, Juraj Linkeš wrote:
<snip>
@@ -723,7 +731,13 @@ def _start_application(self, get_privileged_command:
Callable[[str], str] | None
if self._app_args.app_params is None:
self._app_args.app_params = TestPmdParameters()
- self.number_of_ports = len(self._app_args.ports) if
self._app_args.ports is not None else 0
+ assert isinstance(self._app_args.app_params, TestPmdParameters)
+
This is tricky because ideally we wouldn't have the assertion here,
but I understand why it is needed because Eal parameters have app args
which can be any instance of params. I'm not sure of the best way to
solve this, because making testpmd parameters extend from eal would
break the general scheme that you have in place, and having an
extension of EalParameters that enforces this app_args is
TestPmdParameters would solve the issues, but might be a little
clunky. Is there a way we can use a generic to get python to just
understand that, in this case, this will always be TestPmdParameters?
If not I might prefer making a private class where this is
TestPmdParameters, just because there aren't really any other
assertions that we use elsewhere and an unexpected exception from this
(even though I don't think that can happen) could cause people some
issues.
It might be the case that an assertion is the easiest way to deal with
it though, what do you think?
We could change the signature (just the type of app_args) of the init
method - I think we should be able to create a type that's
EalParameters with .app_params being TestPmdParameters or None. The
init method would just call super().
Something like the above is basically necessary with inheritance where
subclasses are all extensions (not just implementations) of the
superclass (having differences in API).
I believe this is indeed a tricky one. But, unfortunately, I am not
understanding the solution that is being proposed. To me, it just feels
like using a generic factory like:
self.sut_node.create_interactive_shell(..)
is one of the reasons to bring in the majority of these complexities.
What do you mean by creating this new type that combines EalParams and
TestPmdParams?
