The question is when the exception would be raised, or, in other words,
what should we do when hasattr(cls, name) is False. If I understand this
correctly, is it's False, then name is not among the flags and that
means testpmd returned an unsupported flag, which shouldn't happen, but
if it does in the future, we would be better off throwing an exception,
or at very least, log a warning, so that we have an indication that we
need to add support for a new flag.
This is a good point. Realistically if it is ever false that would
mean we have a gap in implementation. I like the idea of flagging a
loud warning over throwing an exception in this case though since if
we threw an exception that would stop all test cases that use OL flags
to stop working even if they don't require the new flag. That would
definitely get the problem fixed sooner, but would also shutdown
automated testing until it then.
It's a tradeoff between risking CI being affected (if a new flag is
added without also adding it to DTS) and how noticable the warning is. I
guess we can implement something in CI that will look for warnings like
these?
+ flag |= cls[name]
+ return flag
+
+ @classmethod
+ def make_parser(cls) -> ParserFn:
+ """Makes a parser function.
+
+ Returns:
+ ParserFn: A dictionary for the `dataclasses.field` metadata
argument containing a
+ parser function that makes an instance of this flag from text.
+ """
+ return TextParser.wrap(
+ TextParser.wrap(TextParser.find(r"ol_flags: ([^\n]+)"), str.split),
+ cls.from_str_list,
+ )
The RSSOffloadTypesFlag does the split in its from_list_string method.
Do we want to do the same here?
Maybe could create a ParsableFlag (or Creatable? Or something else)
superclass that would implement these from_* methods (from_list_string,
from_str) and subclass it. Flags should be subclassable if they don't
contain members.
The superclass would be useful so that we don't redefine the same method
over and over and so that it's clear what's already available.
I like this idea a lot. Basically all of these flags that are used in
parsers are going to need something like that which is going to be
basically the same so just implementing it one time would be great.
I'm not sure if it fits the scope of this series though, do you think
I should write it and add it here or in a separate patch?
A separate patch seems better, as it touches different parts of the
code. We should probably implement the same logic in this patch (without
the exception or warning and with the same if condition) and then make
changes in the other patch.
@@ -656,6 +1147,9 @@ def stop(self, verify: bool = True) -> None:
Raises:
InteractiveCommandExecutionError: If `verify` is :data:`True`
and the command to stop
forwarding results in an error.
+
+ Returns:
+ Output gathered from sending the stop command.
This not just from sending the stop command, but everything else that
preceded (when collecting the verbose output), right?
Technically yes, but that's just due to the nature of how interactive
shells aren't perfect when it comes to asynchronous output. That's why
I tried to be sneaky and say that it is the "output gathered from
sending the stop command" trying to imply that it is not just the
output of the `stop` command, but all the output that is gathered from
sending it. I can update this though.
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
@@ -27,6 +27,12 @@
from .exception import ConfigurationError
REGEX_FOR_PCI_ADDRESS: str =
"/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+_REGEX_FOR_COLON_SEP_MAC: str = r"(?:[\da-fA-F]{2}:){5}[\da-fA-F]{2}"
+_REGEX_FOR_HYPHEN_SEP_MAC: str = r"(?:[\da-fA-F]{2}-){5,7}[\da-fA-F]{2}"
{5,7} should be just 5 repetitions. When could it be more?
I added it for EUI-64 addresses, but maybe this isn't very relevant
here since I just read that they are encouraged on non-ethernet
devices. I can remove it if it doesn't seem worth it to capture.
From what I gather the EUI-64 address is composed from MAC addresses,
but it's a different identifier. I'd say if we ever need it we can add
it as a separate regex (and look for both if we need to).
