chore(agw): Create Python environment with bazel #14266

jheidbrink · 2022-10-25T15:11:00Z

Summary

Uses rules_pyvenv to generate a virtual environment for IDEs. The environment contains PyPI dependencies and compiled Protobuf Python libraries.

Test Plan

In the VM or devcontainer:

sudo apt install python3-venv && cd $MAGMA_ROOT && bazel run //dev_tools:python_env ~/python_ide_env

Now configure the IDE to use the Python interpreter /home/vscode/python_ide_env/bin/python3 (VS Code + Devcontainer) or sftp://vagrant@localhost:2222/home/vagrant/python_ide_env/bin/python3 (IntelliJ IDEA Ultimate + magma_dev VM). You might have to restart the IDE for the change to be effective.

github-actions · 2022-10-25T15:11:20Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

lte/protos/BUILD.bazel

lte/protos/oai/BUILD.bazel

orc8r/protos/BUILD.bazel

github-actions · 2022-10-25T15:28:15Z

feg-workflow

    2 files 203 suites 39s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs 388 ✔️ 0 💤 0 ❌

Results for commit 5ec32fa.

♻️ This comment has been updated with latest results.

jheidbrink · 2022-10-25T15:32:31Z

.devcontainer/devcontainer.json

@@ -82,9 +82,9 @@
 		"python.terminal.activateEnvironment": true,
 		"python.analysis.extraPaths": [
 			"${containerWorkspaceFolder}/orc8r/gateway/python/",
-			"${containerWorkspaceFolder}/lte/gateway/python/",
-			"/home/vscode/build/python/lib/python3.8/site-packages" // has to be in sync with $PYTHON_VENV and $PYTHON_VERSION from .devcontainer/Dockerfile


I think this line was unnecessary - that folder should already be taken into account when the interpreter is set to /home/vscode/build/python/bin/python3.8

Regarding the PR description: the path /home/vscode/python_ide_env/bin/python3 did not work for me when building the env in the VM, but it did work when I built it in the devcontainer. I assume it is due to ~/python_ide_env not being mounted.

github-actions · 2022-10-25T16:00:43Z

dp-workflow

14 tests 14 ✔️ 2m 10s ⏱️
  1 suites   0 💤
  1 files   0 ❌

Results for commit 5ec32fa.

github-actions · 2022-10-25T16:35:45Z

agw-workflow

615 tests 611 ✔️ 4m 41s ⏱️
    2 suites     4 💤
    2 files     0 ❌

Results for commit 5ec32fa.

nstng · 2022-10-28T06:47:10Z

Findings:

in general: works fast and nice!
we could add the python deps that we build our self, i.e., from bazel/external, e.g., ryu, bcc, aioh2, ...
some generated proto libs are missing, e.g., dp/protos, feg/protos
would it be better to have a glob target for the proto libs?
I can not navigate to netifaces (e.g., from lte/gateway/python/magma/pipelined/ifaces.py)
where should we document the setup?

jheidbrink · 2022-10-28T11:23:01Z

would it be better to have a glob target for the proto libs?

I opted for explicitly specifying all the existing targets for two reasons:

We have some targets in //te/protos that reference .proto files from /orc8r/protos. Just sourcing all protos from the lte folder does not do the trick. In contrast the existing targets already get it right.
By using a glob, we would have to specify the united dependencies of all .proto files. The existing targets already correctly specify their dependencies, so by using those we don't have to declare the dependencies twice.

Of course the drawback is that the list doesn't automatically update. I think this is ok given that a change will probably just break a small part of the code navigation at worst.

jheidbrink · 2022-10-28T11:57:19Z

I can not navigate to netifaces (e.g., from lte/gateway/python/magma/pipelined/ifaces.py)

I think that's because netifaces is not written in Python and only available as shared object file. I cannot navigate there using the environment created with make buildenv neither.

sebathomas · 2022-10-28T12:01:06Z

I can not navigate to netifaces (e.g., from lte/gateway/python/magma/pipelined/ifaces.py)

I can navigate there in Intellij but it only shows method interfaces and constants, not implementations. Not sure if VS Code has implemented that feature.

jheidbrink · 2022-10-28T12:20:16Z

Yes, it seems IntelliJ generates a stub Python file. On my system the path is ~/.cache/JetBrains/IntelliJIdea2022.1/python_stubs/287614887/netifaces.py. It seem VS Code doesn't do that.

bazel/external/requirements_README.md

github-actions · 2022-10-28T14:10:08Z

Oops! Looks like you failed the PR Check DCO. Be sure to sign all your commits.

Howto

♻️ Updated: ✅ The check is passing the PR Check DCO after the last commit.

jheidbrink · 2022-10-28T14:20:36Z

bazel/external/requirements_README.md

@@ -10,7 +10,7 @@ Requirements.txt holds all Python dependencies which are required by Python-base

       `cd $MAGMA/bazel/external`

-       `pip-compile --generate-hashes --output-file=requirements.txt requirements.in`
+       `pip-compile --upgrade-package --generate-hashes --output-file=requirements.txt requirements.in`


This is unrelated but I stumbled upon it while working on this PR. The point is that without this parameter, when a requirements.txt already exists and the versions satisfy the constraints from requirements.in, they are not upgraded. With --upgrade-package, versions in an existing requirements.in are upgraded to the highest versions that satisfy the constraints.

jheidbrink · 2022-10-28T14:27:41Z

.devcontainer/Dockerfile

@@ -206,74 +207,4 @@ RUN GOBIN="/usr/bin/" go install -v golang.org/x/tools/gopls@v0.8.3
 #### Update shared library configuration
 RUN ldconfig -v

-


Instead of the removed block, we will use the Python environment built by Bazel. This requires us to build that environment initially - I'll update https://github.com/magma/magma/wiki/Contributing-Code-with-VSCode and https://github.com/magma/magma/wiki/Contributing-Code-with-IntelliJ once this gets merged.

While having to build the Python environment is inconvenient, this reduces the devcontainer download size by 175 MB. Given that not all contributors are working on Python code, that seems to be a good tradeoff.

jheidbrink · 2022-10-28T14:33:23Z

we could add the python deps that we build our self, i.e., from bazel/external, e.g., ryu, bcc, aioh2, ...

some generated proto libs are missing, e.g., dp/protos, feg/protos

Added protos and external Python deps (the latter in a hacky way unfortunately).

where should we document the setup?

I'll update the VS Code section and the IntelliJ section in the Wiki once this gets merged.

LKreutzer

Tested locally

LKreutzer · 2022-10-31T10:16:57Z

.devcontainer/devcontainer.json

@@ -82,9 +82,9 @@
 		"python.terminal.activateEnvironment": true,
 		"python.analysis.extraPaths": [
 			"${containerWorkspaceFolder}/orc8r/gateway/python/",
-			"${containerWorkspaceFolder}/lte/gateway/python/",
-			"/home/vscode/build/python/lib/python3.8/site-packages" // has to be in sync with $PYTHON_VENV and $PYTHON_VERSION from .devcontainer/Dockerfile


Regarding the PR description: the path /home/vscode/python_ide_env/bin/python3 did not work for me when building the env in the VM, but it did work when I built it in the devcontainer. I assume it is due to ~/python_ide_env not being mounted.

Signed-off-by: Jan Heidbrink <jan.heidbrink@tngtech.com>

jheidbrink · 2022-10-31T15:06:41Z

Updates to the Wiki: https://github.com/magma/magma/wiki/Contributing-Code-with-VSCode/_compare/5d9776237920cb6b783e0ecfc0a018d9d4d23a16...fae65bd545ca29bd7a680002bbb48b054db96fc9 and https://github.com/magma/magma/wiki/Contributing-Code-with-IntelliJ/_compare/6e50540ed305dcf08014de382571e9f6358f2418...136bd3608b033e3add450e6c27d4724f9147a82e

Signed-off-by: Jan Heidbrink <jan.heidbrink@tngtech.com>

jheidbrink requested a review from a team October 25, 2022 15:11

jheidbrink requested a review from a team as a code owner October 25, 2022 15:11

jheidbrink requested review from nstng and ardzoht October 25, 2022 15:11

pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Oct 25, 2022

github-actions bot added component: agw Access gateway-related issue component: orc8r Orchestrator-related issue labels Oct 25, 2022

github-actions bot reviewed Oct 25, 2022

View reviewed changes

lte/protos/BUILD.bazel Outdated Show resolved Hide resolved

lte/protos/oai/BUILD.bazel Outdated Show resolved Hide resolved

orc8r/protos/BUILD.bazel Outdated Show resolved Hide resolved

jheidbrink added the bazel changes for the Bazelification effort label Oct 25, 2022

jheidbrink force-pushed the bazel_python_ide branch from 263bdb1 to 3a5509e Compare October 25, 2022 15:13

jheidbrink force-pushed the bazel_python_ide branch from d022cd4 to 5ec32fa Compare October 25, 2022 15:29

github-actions bot removed the component: orc8r Orchestrator-related issue label Oct 25, 2022

jheidbrink commented Oct 25, 2022

View reviewed changes

jheidbrink requested review from LKreutzer and voisey October 26, 2022 14:18

github-actions bot reviewed Oct 28, 2022

View reviewed changes

bazel/external/requirements_README.md Show resolved Hide resolved

jheidbrink commented Oct 28, 2022

View reviewed changes

jheidbrink force-pushed the bazel_python_ide branch from d44da39 to fc6487f Compare October 28, 2022 14:35

jheidbrink force-pushed the bazel_python_ide branch from fc6487f to e07bca1 Compare October 28, 2022 14:36

LKreutzer approved these changes Oct 31, 2022

View reviewed changes

ajahl approved these changes Oct 31, 2022

View reviewed changes

jheidbrink force-pushed the bazel_python_ide branch from e07bca1 to d25187a Compare October 31, 2022 10:43

chore(agw): Create Python environment with bazel

c4a3fcb

Signed-off-by: Jan Heidbrink <jan.heidbrink@tngtech.com>

jheidbrink force-pushed the bazel_python_ide branch from d25187a to c4a3fcb Compare October 31, 2022 13:12

jheidbrink enabled auto-merge (squash) October 31, 2022 13:12

jheidbrink merged commit 85b9025 into magma:master Oct 31, 2022

lucasgonze pushed a commit to lucasgonze/magma that referenced this pull request Feb 29, 2024

chore(agw): Create Python environment with bazel (magma#14266)

85d4a3f

Signed-off-by: Jan Heidbrink <jan.heidbrink@tngtech.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(agw): Create Python environment with bazel #14266

chore(agw): Create Python environment with bazel #14266

jheidbrink commented Oct 25, 2022 •

edited

github-actions bot commented Oct 25, 2022

github-actions bot commented Oct 25, 2022 •

edited

jheidbrink Oct 25, 2022

LKreutzer Oct 31, 2022 •

edited

github-actions bot commented Oct 25, 2022

github-actions bot commented Oct 25, 2022

nstng commented Oct 28, 2022

jheidbrink commented Oct 28, 2022

jheidbrink commented Oct 28, 2022

sebathomas commented Oct 28, 2022

jheidbrink commented Oct 28, 2022

github-actions bot commented Oct 28, 2022 •

edited

jheidbrink Oct 28, 2022

jheidbrink Oct 28, 2022 •

edited

jheidbrink commented Oct 28, 2022

LKreutzer left a comment

LKreutzer Oct 31, 2022 •

edited

jheidbrink commented Oct 31, 2022

		@@ -206,74 +207,4 @@ RUN GOBIN="/usr/bin/" go install -v golang.org/x/tools/gopls@v0.8.3
		#### Update shared library configuration
		RUN ldconfig -v

chore(agw): Create Python environment with bazel #14266

chore(agw): Create Python environment with bazel #14266

Conversation

jheidbrink commented Oct 25, 2022 • edited

Summary

Test Plan

github-actions bot commented Oct 25, 2022

Howto

More info

github-actions bot commented Oct 25, 2022 • edited

feg-workflow

jheidbrink Oct 25, 2022

Choose a reason for hiding this comment

LKreutzer Oct 31, 2022 • edited

Choose a reason for hiding this comment

github-actions bot commented Oct 25, 2022

dp-workflow

github-actions bot commented Oct 25, 2022

agw-workflow

nstng commented Oct 28, 2022

jheidbrink commented Oct 28, 2022

jheidbrink commented Oct 28, 2022

sebathomas commented Oct 28, 2022

jheidbrink commented Oct 28, 2022

github-actions bot commented Oct 28, 2022 • edited

Howto

jheidbrink Oct 28, 2022

Choose a reason for hiding this comment

jheidbrink Oct 28, 2022 • edited

Choose a reason for hiding this comment

jheidbrink commented Oct 28, 2022

LKreutzer left a comment

Choose a reason for hiding this comment

LKreutzer Oct 31, 2022 • edited

Choose a reason for hiding this comment

jheidbrink commented Oct 31, 2022

jheidbrink commented Oct 25, 2022 •

edited

github-actions bot commented Oct 25, 2022 •

edited

LKreutzer Oct 31, 2022 •

edited

github-actions bot commented Oct 28, 2022 •

edited

jheidbrink Oct 28, 2022 •

edited

LKreutzer Oct 31, 2022 •

edited