Both good points, I'll be sure to add/adjust those docstrings. On Thu, Nov 16, 2023 at 1:05 PM Juraj Linkeš <juraj.lin...@pantheon.tech> wrote:
> On Mon, Nov 13, 2023 at 9:28 PM <jspew...@iol.unh.edu> wrote: > > > > From: Jeremy Spewock <jspew...@iol.unh.edu> > > > > Adds methods in both os_session and linux session to allow for setting > > MTU of port interfaces in an OS agnostic way. > > > > Signed-off-by: Jeremy Spewock <jspew...@iol.unh.edu> > > --- > > dts/framework/remote_session/linux_session.py | 7 +++++++ > > dts/framework/remote_session/os_session.py | 9 +++++++++ > > 2 files changed, 16 insertions(+) > > > > diff --git a/dts/framework/remote_session/linux_session.py > b/dts/framework/remote_session/linux_session.py > > index a3f1a6bf3b..dab68d41b1 100644 > > --- a/dts/framework/remote_session/linux_session.py > > +++ b/dts/framework/remote_session/linux_session.py > > @@ -196,6 +196,13 @@ def configure_port_ip_address( > > verify=True, > > ) > > > > + def configure_port_mtu(self, mtu: int, port: Port) -> None: > > This is missing a docstring. > The way I've decided to document these overridden abstract methods is > to just to link to the superclass's method, used heavily in > > https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-17-juraj.lin...@pantheon.tech/ > : > """Overrides :meth:`~.os_session.OSSession.configure_port_mtu`.""" > > The docstring checker complains if the docstring is missing. There may > be better ways, such as with @typing.override (but that requires > Python 3.12 or typing_extensions, but there's a bug in Pylama that > prevents us from using that solution: > https://github.com/klen/pylama/pull/247). > > > + self.send_command( > > + f"ip link set dev {port.logical_name} mtu {mtu}", > > + privileged=True, > > + verify=True, > > + ) > > + > > def configure_ipv4_forwarding(self, enable: bool) -> None: > > state = 1 if enable else 0 > > self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", > privileged=True) > > diff --git a/dts/framework/remote_session/os_session.py > b/dts/framework/remote_session/os_session.py > > index 8a709eac1c..c038f78b79 100644 > > --- a/dts/framework/remote_session/os_session.py > > +++ b/dts/framework/remote_session/os_session.py > > @@ -277,6 +277,15 @@ def configure_port_ip_address( > > Configure (add or delete) an IP address of the input port. > > """ > > > > + @abstractmethod > > + def configure_port_mtu(self, mtu: int, port: Port) -> None: > > + """Configure MTU on a given port. > > + > > + Args: > > + mtu: Desired MTU value. > > + port: Port to set the MTU on. > > + """ > > I've compiled the rules for composing docstrings here: > > https://patches.dpdk.org/project/dpdk/patch/20231115130959.39420-4-juraj.lin...@pantheon.tech/ > . > > The relevant part here is: > > When referencing a parameter of a function or a method in their > docstring, don't use > any articles and put the parameter into single backticks. This mimics > the style of > `Python's documentation <https://docs.python.org/3/index.html>`_. > > Both the mtu and the port parameters are mentioned, so they should be > without articles and in backticks. > > > + > > @abstractmethod > > def configure_ipv4_forwarding(self, enable: bool) -> None: > > """ > > -- > > 2.42.0 > > >
