From: Owen Hilyard <ohilyard@iol.unh.edu>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "Juraj Linkeš" <juraj.linkes@pantheon.tech>,
Honnappa.Nagarahalli@arm.com, lijuan.tu@intel.com,
kda@semihalf.com, bruce.richardson@intel.com, dev@dpdk.org
Subject: Re: [PATCH v6 00/10] dts: ssh connection to a node
Date: Wed, 2 Nov 2022 08:58:41 -0400 [thread overview]
Message-ID: <CAHx6DYBSxEkUzwizs5c=B8kL9z-6hXadq99vhVGPM07xVjOvqw@mail.gmail.com> (raw)
In-Reply-To: <5711158.zQ0Gbyo6oJ@thomas>
[-- Attachment #1: Type: text/plain, Size: 7289 bytes --]
On Mon, Oct 31, 2022 at 3:01 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> I was about to merge this series,
> and after long thoughts, it deserves a bit more changes.
> I would like to work with you for a merge in 22.11-rc3.
>
> 13/10/2022 12:35, Juraj Linkeš:
> > All the necessary code needed to connect to a node in a topology with
> > a bit more, such as basic logging and some extra useful methods.
>
> There is also some developer tooling,
> and some documentation.
>
> [...]
> > There are configuration files with a README that help with setting up
> > the execution/development environment.
>
> I don't want to merge some doc which is not integrated
> in the doc/ directory.
> It should be in RST format in doc/guides/dts/
> I can help with this conversion.
>
> > The code only connects to a node. You'll see logs emitted to console
> > saying where DTS connected.
> >
> > There's only a bit of documentation, as there's not much to document.
> > We'll add some real docs when there's enough functionality to document,
> > when the HelloWorld testcases is in (point 4 in our roadmap below). What
> > will be documented later is runtime dependencies and how to set up the
> DTS
> > control node environment.
> >
> [...]
> > .editorconfig | 2 +-
> > .gitignore | 9 +-
>
> Updating general Python guidelines in these files
> should be done separately to get broader agreement.
>
> > MAINTAINERS | 5 +
>
> You can update this file in the first patch.
>
> > devtools/python-checkpatch.sh | 39 ++
>
> Let's postpone the integration of checkpatch.
> It should be integrated with the existing checkpatch.
>
We wanted to specifically ensure that all code met the quality requirements
for DTS from the start. The current formatting requirements are "whatever
these tools run in this order with these settings results in", since the
working group decided that fully automated rectification of minor issues
would help new contributors focus on more important things. I agree that
integrating it into existing checkpatch would be good, but I think that to
do that we would first need to run the tool over existing DPDK python code.
python-checkpatch does do linting like the main checkpatch, but it will
also perform source code changes to automatically fix many formatting
issues. Do we want to keep checkpatch as a read-only script and introduce
another one which makes source-code changes, or merge them together? This
would also mean that all DPDK developers would need to run checkpatch
inside of a python virtual environment since DTS is currently very specific
about what versions are used (via both version number and cryptographic
hash of the source archive). These versions are newer than what are shipped
in many stable linux distributions (Ubuntu, Debian, etc), because we want
the additional capabilities that come with the newer versions and the
version in the Debian Buster repos is old enough that it does not support
the version of python that we are using. We were trying to avoid forcing
all DPDK developers to install a full suite of python tools.
> devtools/python-format.sh | 54 +++
> > devtools/python-lint.sh | 26 ++
>
> Let's postpone the integration of these tools.
> We need to discuss what is specific to DTS or not.
>
> > doc/guides/contributing/coding_style.rst | 4 +-
>
> It is not specific to DTS.
>
> > dts/.devcontainer/devcontainer.json | 30 ++
> > dts/Dockerfile | 39 ++
>
> Not sure about Docker tied to some personal choices.
>
The reason we are using docker is that it provides a canonical environment
to run the test harness in, which can then connect to the SUT and traffic
generator. It enforces isolation between these three components, and
ensures that everyone is using the exact same environment to test and
deploy the test harness. Although devcontainer started in Visual Studio
Code, it is supported by many IDEs at this point and we wanted to encourage
developers along a path which avoids needing to set up a development
environment before they can start working. The very recent version of
python (3.10) we choose to use for additional type system verification
makes not using containers much harder. It also means that new
dependencies, new dependency versions or additional system dependencies can
be easily handled by the normal patch process and rolled out to everyone in
a mostly automated fashion.
Additionally, although it is named Dockerfile, it is fully compatible with
podman.
> > dts/README.md | 154 ++++++++
>
> As said above, it should in RST format in doc/guides/dts/
>
> > dts/conf.yaml | 6 +
> > dts/framework/__init__.py | 4 +
> > dts/framework/config/__init__.py | 100 +++++
> > dts/framework/config/conf_yaml_schema.json | 65 ++++
> > dts/framework/dts.py | 68 ++++
> > dts/framework/exception.py | 57 +++
> > dts/framework/logger.py | 114 ++++++
> > dts/framework/remote_session/__init__.py | 15 +
> > .../remote_session/remote_session.py | 100 +++++
> > dts/framework/remote_session/ssh_session.py | 185 +++++++++
> > dts/framework/settings.py | 119 ++++++
> > dts/framework/testbed_model/__init__.py | 8 +
> > dts/framework/testbed_model/node.py | 63 ++++
> > dts/framework/utils.py | 31 ++
> > dts/main.py | 24 ++
> > dts/poetry.lock | 351 ++++++++++++++++++
>
> A lot of dependencies look not useful in this first series for SSH
> connection.
>
I've put the actual dependency list below. The lock file contains the
entire dependency tree, of which Scapy is responsible for a substantial
portion. pexpect is currently used because of the large amount of code from
the old dts that was moved over, but it will be replaced by a better
solution soon with a rewrite of the dts/framework/remote_session module.
Warlock is used to handle json schema checking, which is how we provide
instant feedback about an incorrect config file to users (as opposed to old
DTS, where you would be informed by a stack trace at an arbitrary point in
the program). PyYAML is used to parse the yaml config file, and
types-PyYAML is used to provide information to the python type checker
about pyyaml. Scapy provides all of the packet manipulation capabilities in
DTS, and I was not able to find a better library for doing packet
manipulation in python, so in my eyes we can excuse its dependency tree due
to how much time it will save.
[tool.poetry.dependencies]
python = "^3.10"
pexpect = "^4.8.0"
warlock = "^2.0.1"
PyYAML = "^6.0"
types-PyYAML = "^6.0.8"
scapy = "^2.4.5"
>
> > dts/pyproject.toml | 55 +++
> > 27 files changed, 1723 insertions(+), 4 deletions(-)
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 8821 bytes --]
next prev parent reply other threads:[~2022-11-02 12:59 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-13 10:35 Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 01/10] dts: add project tools config Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 02/10] dts: add developer tools Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 03/10] dts: add config parser module Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 04/10] dts: add basic logging facility Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 05/10] dts: add remote session abstraction Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 06/10] dts: add ssh session module Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 07/10] dts: add node base class Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 08/10] dts: add dts workflow module Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 09/10] dts: add dts executable script Juraj Linkeš
2022-10-13 10:35 ` [PATCH v6 10/10] maintainers: add dts maintainers Juraj Linkeš
2022-10-13 10:45 ` [PATCH v6 00/10] dts: ssh connection to a node Bruce Richardson
2022-10-31 19:01 ` Thomas Monjalon
2022-11-02 12:58 ` Owen Hilyard [this message]
2022-11-02 13:15 ` Thomas Monjalon
2022-11-03 15:19 ` [PATCH v7 0/9] " Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 1/9] dts: add project tools config Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 2/9] dts: add developer tools Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 3/9] dts: add config parser module Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 4/9] dts: add basic logging facility Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 5/9] dts: add remote session abstraction Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 6/9] dts: add ssh session module Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 7/9] dts: add node base class Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 8/9] dts: add dts workflow module Juraj Linkeš
2022-11-03 15:19 ` [PATCH v7 9/9] dts: add dts executable script Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 0/9] dts: ssh connection to a node Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 1/9] dts: add project tools config Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 2/9] dts: add developer tools Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 3/9] dts: add config parser module Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 4/9] dts: add basic logging facility Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 5/9] dts: add remote session abstraction Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 6/9] dts: add ssh session module Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 7/9] dts: add node base class Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 8/9] dts: add dts workflow module Juraj Linkeš
2022-11-04 11:05 ` [PATCH v8 9/9] dts: add dts executable script Juraj Linkeš
2022-11-09 16:11 ` [PATCH v8 0/9] dts: ssh connection to a node Thomas Monjalon
2022-11-09 16:23 ` Honnappa Nagarahalli
2022-11-09 17:05 ` Owen Hilyard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHx6DYBSxEkUzwizs5c=B8kL9z-6hXadq99vhVGPM07xVjOvqw@mail.gmail.com' \
--to=ohilyard@iol.unh.edu \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=juraj.linkes@pantheon.tech \
--cc=kda@semihalf.com \
--cc=lijuan.tu@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).