02/11/2022 13:58, Owen Hilyard: > On Mon, Oct 31, 2022 at 3:01 PM Thomas Monjalon <tho...@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.
So who is running the script? My wish is to make it automatically run when calling checkpatch.sh. > > 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. In general we avoid enforcing specific versions in DPDK. The idea is to make it usable from any system with good compatibility. Using a container is more convenient in general, I agree. I just would like to have the choices here discussed specifically in a separate patch. > > > 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" Scapy is not needed for SSH connection. Is warlock used in this series?
