DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: <thomas@monjalon.net>, <david.marchand@redhat.com>,
	<ronan.randles@intel.com>, <Honnappa.Nagarahalli@arm.com>,
	<ohilyard@iol.unh.edu>, <lijuan.tu@intel.com>, <dev@dpdk.org>
Subject: Re: [PATCH v4 2/9] dts: add developer tools
Date: Wed, 7 Sep 2022 17:37:27 +0100	[thread overview]
Message-ID: <YxjIx/47s8yG4nkJ@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20220729105550.1382664-3-juraj.linkes@pantheon.tech>

On Fri, Jul 29, 2022 at 10:55:43AM +0000, Juraj Linkeš wrote:
> The Dockerfile contains basic image for CI and developers. There's also
> an integration of the Dockerfile with Visual Studio.
> 
> The formatter script uses Black and Isort to format the Python code.
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Comments inline below.

Thanks,
/Bruce

> ---
>  dts/.devcontainer/devcontainer.json | 30 ++++++++++++
>  dts/Dockerfile                      | 38 +++++++++++++++
>  dts/README.md                       | 74 ++++++++++++++++++++++++++++-
>  dts/format.sh                       | 45 ++++++++++++++++++
>  4 files changed, 186 insertions(+), 1 deletion(-)
>  create mode 100644 dts/.devcontainer/devcontainer.json
>  create mode 100644 dts/Dockerfile
>  create mode 100755 dts/format.sh
> 
> diff --git a/dts/.devcontainer/devcontainer.json b/dts/.devcontainer/devcontainer.json
> new file mode 100644
> index 0000000000..41ca28fc17
> --- /dev/null
> +++ b/dts/.devcontainer/devcontainer.json
> @@ -0,0 +1,30 @@
> +// For format details, see https://aka.ms/devcontainer.json. For config options, see the README at:
> +// https://github.com/microsoft/vscode-dev-containers/tree/v0.241.1/containers/docker-existing-dockerfile
> +{
> +	"name": "Existing Dockerfile",
> +
> +	// Sets the run context to one level up instead of the .devcontainer folder.
> +	"context": "..",
> +
> +	// Update the 'dockerFile' property if you aren't using the standard 'Dockerfile' filename.
> +	"dockerFile": "../Dockerfile",
> +
> +	// Use 'forwardPorts' to make a list of ports inside the container available locally.
> +	// "forwardPorts": [],
> +
> +	// Uncomment the next line to run commands after the container is created - for example installing curl.
> +	"postCreateCommand": "poetry install",
> +
> +	"extensions": [
> +		"ms-python.vscode-pylance",
> +	]
> +
> +	// Uncomment when using a ptrace-based debugger like C++, Go, and Rust
> +	// "runArgs": [ "--cap-add=SYS_PTRACE", "--security-opt", "seccomp=unconfined" ],
> +
> +	// Uncomment to use the Docker CLI from inside the container. See https://aka.ms/vscode-remote/samples/docker-from-docker.
> +	// "mounts": [ "source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind" ],
> +
> +	// Uncomment to connect as a non-root user if you've added one. See https://aka.ms/vscode-remote/containers/non-root.
> +	// "remoteUser": "vscode"
> +}
> diff --git a/dts/Dockerfile b/dts/Dockerfile
> new file mode 100644
> index 0000000000..6700aa45b8
> --- /dev/null
> +++ b/dts/Dockerfile
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 University of New Hampshire
> +#
> +
> +FROM ubuntu:22.04 AS base
> +
> +RUN apt-get -y update && apt-get -y upgrade && \
> +    apt-get -y install --no-install-recommends \
> +        python3 \
> +        python3-pip \
> +        python3-pexpect \
> +        python3-poetry \
> +        python3-cachecontrol \
> +        openssh-client
> +
> +
> +FROM base AS runner
> +
> +# This container is intended to be used as the base for automated systems.
> +# It bakes DTS into the container during the build.
> +
> +RUN mkdir /dts
> +COPY ./pyproject.toml /dts/pyproject.toml
> +COPY ./poetry.lock /dts/poetry.lock
> +WORKDIR /dts
> +RUN poetry install --no-dev
> +COPY . /dts

Two questions here:
* if we copy over the current folder, does it re-copy the same two files
  above, or do we get a new subfolder with the same name as the current
  one (and the two files in that instead)?
* Can the commands be re-ordered so that we have all the copies together
  rather than being split either side of the workdir and run commands?

> +
> +CMD ["poetry", "run", "python", "main.py"]
> +
> +FROM base AS dev
> +
> +# This container is intended to be used as a development environment.
> +
> +RUN apt-get -y install --no-install-recommends \
> +        vim emacs git
> +
If it's to be used as a development environment, do we not need
build-essential installed?

> +WORKDIR /dts

Is this needed twice in the file, since it appears above too?

> diff --git a/dts/README.md b/dts/README.md
> index d8f88f97fe..55a272d767 100644
> --- a/dts/README.md
> +++ b/dts/README.md
> @@ -12,4 +12,76 @@ The Python Version required by DTS is specified in
>  [DTS python config file](./pyproject.toml) in the **[tool.poetry.dependencies]**
>  section. Poetry doesn't install Python, so you may need to satisfy this requirement if
>  your Python is not up to date. A tool such as [Pyenv](https://github.com/pyenv/pyenv)
> -is a good way to get Python, though not the only one.
> +is a good way to get Python, though not the only one. However, DTS includes a
> +development environment in the form of a Docker image.
> +
> +# Expected Environment
> +
> +The expected execution and development environments for DTS are the same,
> +the container defined by [Dockerfile](./Dockerfile). Using a container for the
> +development environment helps with a few things.
> +
> +1. It helps enforce the boundary between the tester and the traffic
> +   generator/sut, something which has experienced issues in the past.

s/experienced/caused/

> +2. It makes creating containers to run DTS inside automated tooling
> +   much easier, since they can be based off of a known-working environment
> +   that will be updated as DTS is.
> +3. It abstracts DTS from the server it is running on. This means that the
> +   bare-metal os can be whatever corporate policy or your personal preferences
> +   dictate, and DTS does not have to try to support all 15 distros that
> +   are supported by DPDK CI.

Remove the "15".

> +4. It makes automated testing for DTS easier, since new dependencies can be
> +   sent in with the patches.
> +5. It fixes the issue of undocumented dependencies, where some test suites
> +   require python libraries that are not installed.
> +6. Allows everyone to use the same python version easily, even if they are
> +   using an LTS distro or Windows.

Presumably the LTS distro is an *older* LTS distribution with possibly
out-of-date packages? That should perhaps be made clearer.

> +7. Allows you to run the tester on Windows while developing via Docker for
> +   Windows.
> +
> +## Tips for setting up a development environment
> +
> +### Getting a docker shell
> +
> +These commands will give you a bash shell inside the container with all the python
> +dependencies installed. This will place you inside a python virtual
> +environment. DTS is mounted via a volume, which is essentially a symlink
> +from the host to the container. This enables you to edit and run inside the container
> +and then delete the container when you are done, keeping your work.
> +
> +```shell
> +docker build --target dev -t dpdk-dts .
> +docker run -v $(pwd):/dts -it dpdk-dts bash
> +$ poetry install
> +$ poetry shell
> +```
> +
> +### Vim/Emacs
> +
> +Any editor in the ubuntu repos should be easy to use. You can add your normal
> +config files as a volume, enabling you to use your preferred settings.
> +
> +```shell
> +apt install vim
> +apt install emacs
> +```

Were these not already installed in the image created using the dockerfile
above?

> +
> +### Visual Studio Code
> +
> +VSCode has first-class support for developing with containers. You may need to run the
> +non-docker setup commands in the integrated terminal. DTS contains a .devcontainer
> +config, so if you open the folder in vscode it should prompt you to use the dev
> +container assuming you have the plugin installed. Please refer to
> +[VS Development Containers Docs](https://code.visualstudio.com/docs/remote/containers)
> +to set it all up.
> +
> +### Other
> +
> +Searching for '$IDE dev containers' will probably lead you in the right
> +direction.
> +
> +# Python Formatting
> +
> +The tools used to format Python code in DTS are Black and Isort. There's a shell
> +script, function.sh, which runs the formatters. Poetry will install these tools,
> +so once you have that set up, you should run it before submitting patches.
> diff --git a/dts/format.sh b/dts/format.sh
> new file mode 100755
> index 0000000000..7d72335470
> --- /dev/null
> +++ b/dts/format.sh
> @@ -0,0 +1,45 @@
> +#!/usr/bin/env bash
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 University of New Hampshire
> +# Copyright(c) 2022 PANTHEON.tech s.r.o.
> +#
> +
> +function main() {
> +    # The directory to work on is either passed in as argument 1,
> +    # or is the current working directory
> +    DIRECTORY=${1:-$(pwd)}
> +    LINE_LENGTH=88
> +
> +    BLACK_VERSION=$(awk '/\[tool.poetry.dev-dependencies\]/,/$^/' pyproject.toml |\
> +                    grep black | grep -o '[0-9][^"]*')
> +
> +    PYTHON_VERSION=$(awk '/\[tool.poetry.dependencies\]/,/$^/' pyproject.toml |\
> +                    grep python | grep -o '[0-9][^"]*' | tr -d '.')
> +
> +    isort \
> +      --overwrite-in-place \
> +      --profile black \
> +      -j "$(nproc)" \
> +      --line-length $LINE_LENGTH \
> +      --python-version auto \
> +      "$DIRECTORY"
> +
> +    black \
> +      --line-length $LINE_LENGTH \
> +      --required-version "${BLACK_VERSION}" \
> +      --target-version "py${PYTHON_VERSION}" \
> +      --safe \
> +      "$DIRECTORY"
> +}
> +
> +function help() {
> +  echo "usage: format.sh <directory>"
> +}
> +
> +if [ "$1" == "-h" ] || [ "$1" == "--help" ]; then
> +  help
> +  exit 0
> +fi
> +
> +main "$1"
> +
> -- 
> 2.30.2
> 

  parent reply	other threads:[~2022-09-07 16:37 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-22 12:14 [PATCH v1 0/8] dts: ssh connection to a node Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 1/8] dts: add ssh pexpect library Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 2/8] dts: add locks for parallel node connections Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 3/8] dts: add ssh connection extension Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 4/8] dts: add basic logging facility Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 5/8] dts: add Node base class Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 6/8] dts: add config parser module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 7/8] dts: add dts runtime workflow module Juraj Linkeš
2022-06-22 12:14 ` [PATCH v1 8/8] dts: add main script for running dts Juraj Linkeš
2022-07-11 14:51 ` [PATCH v2 0/8] ssh connection to a node Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 1/8] dts: add basic logging facility Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 2/8] dts: add ssh pexpect library Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 3/8] dts: add locks for parallel node connections Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 4/8] dts: add ssh connection extension Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 5/8] dts: add config parser module Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 6/8] dts: add Node base class Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 7/8] dts: add dts workflow module Juraj Linkeš
2022-07-11 14:51   ` [PATCH v2 8/8] dts: add dts executable script Juraj Linkeš
2022-07-28 10:00   ` [PATCH v3 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 1/9] dts: add project tools config Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 2/9] dts: add developer tools Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 3/9] dts: add basic logging facility Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 5/9] dts: add ssh connection extension Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 6/9] dts: add config parser module Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 7/9] dts: add Node base class Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 8/9] dts: add dts workflow module Juraj Linkeš
2022-07-28 10:00     ` [PATCH v3 9/9] dts: add dts executable script Juraj Linkeš
2022-07-29 10:55     ` [PATCH v4 0/9] dts: ssh connection to a node Juraj Linkeš
2022-07-29 10:55       ` [PATCH v4 1/9] dts: add project tools config Juraj Linkeš
2022-08-10  6:30         ` Tu, Lijuan
2022-09-07 16:16         ` Bruce Richardson
2022-09-09 13:38           ` Juraj Linkeš
2022-09-09 13:52             ` Bruce Richardson
2022-09-09 14:13               ` Juraj Linkeš
2022-09-12 14:06                 ` Owen Hilyard
2022-09-12 15:15                   ` Bruce Richardson
2022-09-13 12:08                     ` Juraj Linkeš
2022-09-13 14:18                       ` Bruce Richardson
2022-09-13 19:03                     ` Honnappa Nagarahalli
2022-09-13 19:19                 ` Honnappa Nagarahalli
2022-09-14  9:37                   ` Thomas Monjalon
2022-09-14 12:55                     ` Juraj Linkeš
2022-09-14 13:11                       ` Bruce Richardson
2022-09-14 14:28                         ` Thomas Monjalon
2022-09-21 10:49                           ` Juraj Linkeš
2022-09-13 19:11             ` Honnappa Nagarahalli
2022-07-29 10:55       ` [PATCH v4 2/9] dts: add developer tools Juraj Linkeš
2022-08-10  6:30         ` Tu, Lijuan
2022-09-07 16:37         ` Bruce Richardson [this message]
2022-09-13 12:38           ` Juraj Linkeš
2022-09-13 20:38             ` Honnappa Nagarahalli
2022-09-14  7:37               ` Bruce Richardson
2022-09-14 12:45               ` Juraj Linkeš
2022-09-14 13:13                 ` Bruce Richardson
2022-09-14 14:26                   ` Thomas Monjalon
2022-09-14 19:08                     ` Honnappa Nagarahalli
2022-09-20 12:14                       ` Juraj Linkeš
2022-09-20 12:22                         ` Tu, Lijuan
2022-07-29 10:55       ` [PATCH v4 3/9] dts: add basic logging facility Juraj Linkeš
2022-08-10  6:31         ` Tu, Lijuan
2022-09-08  8:31         ` Bruce Richardson
2022-09-13 12:52           ` Juraj Linkeš
2022-09-13 23:31             ` Honnappa Nagarahalli
2022-09-14 12:51               ` Juraj Linkeš
2022-07-29 10:55       ` [PATCH v4 4/9] dts: add ssh pexpect library Juraj Linkeš
2022-08-10  6:31         ` Tu, Lijuan
2022-09-08  9:53         ` Bruce Richardson
2022-09-13 13:36           ` Juraj Linkeš
2022-09-13 14:23             ` Bruce Richardson
2022-09-13 14:59         ` Stanislaw Kardach
2022-09-13 17:23           ` Owen Hilyard
2022-09-14  0:03             ` Honnappa Nagarahalli
2022-09-14  7:42               ` Bruce Richardson
2022-09-14  7:58                 ` Stanislaw Kardach
2022-09-14 19:57                 ` Honnappa Nagarahalli
2022-09-19 14:21                   ` Owen Hilyard
2022-09-20 17:54                     ` Honnappa Nagarahalli
2022-09-21  1:01                       ` Tu, Lijuan
2022-09-21  5:37                       ` Jerin Jacob
2022-09-22  9:03                         ` Juraj Linkeš
2022-09-14  9:42         ` Stanislaw Kardach
2022-09-22  9:41           ` Juraj Linkeš
2022-09-22 14:32             ` Stanislaw Kardach
2022-09-23  7:22               ` Juraj Linkeš
2022-09-23  8:15                 ` Bruce Richardson
2022-09-23 10:18                   ` Stanislaw Kardach
2022-07-29 10:55       ` [PATCH v4 5/9] dts: add ssh connection extension Juraj Linkeš
2022-08-10  6:32         ` Tu, Lijuan
2022-09-13 17:04         ` Bruce Richardson
2022-09-13 17:32           ` Owen Hilyard
2022-09-14  7:46             ` Bruce Richardson
2022-09-14 12:02               ` Owen Hilyard
2022-09-14 13:15                 ` Bruce Richardson
2022-07-29 10:55       ` [PATCH v4 6/9] dts: add config parser module Juraj Linkeš
2022-08-10  6:33         ` Tu, Lijuan
2022-09-13 17:19         ` Bruce Richardson
2022-09-13 17:47           ` Owen Hilyard
2022-09-14  7:48             ` Bruce Richardson
2022-07-29 10:55       ` [PATCH v4 7/9] dts: add Node base class Juraj Linkeš
2022-08-10  6:33         ` Tu, Lijuan
2022-07-29 10:55       ` [PATCH v4 8/9] dts: add dts workflow module Juraj Linkeš
2022-08-10  6:34         ` Tu, Lijuan
2022-07-29 10:55       ` [PATCH v4 9/9] dts: add dts executable script Juraj Linkeš
2022-08-10  6:35         ` Tu, Lijuan

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=YxjIx/47s8yG4nkJ@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=lijuan.tu@intel.com \
    --cc=ohilyard@iol.unh.edu \
    --cc=ronan.randles@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).