Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PDDF] Build and install Python 3 package #6286

Merged
merged 7 commits into from
Jan 7, 2021
Merged

[PDDF] Build and install Python 3 package #6286

merged 7 commits into from
Jan 7, 2021

Conversation

jleveque
Copy link
Contributor

- Why I did it

As part of migrating the SONiC codebase from Python 2 to Python 3

- How I did it

  • Make PDDF code compliant with both Python 2 and Python 3

  • Align code with PEP8 standards using autopep8

  • Build and install both Python 2 and Python 3 PDDF packages

  • Once all vendors build and install Python 3 versions of their sonic-platform package, we can simply stop building and installing the Python 2 version of the package.

- How to verify it

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@jleveque jleveque marked this pull request as draft December 23, 2020 22:47
@jleveque
Copy link
Contributor Author

@fk410167: Can you please review and test this PR?

@FuzailBrcm
Copy link
Contributor

@jleveque
There is a build failure and some more changes are required. I resolved the build breakage locally. Build is in progress. Once done, I will test PDDF and update you.

@FuzailBrcm
Copy link
Contributor

@jleveque

I tested the build and found these changes are required for successful build. Please add them.

git diff platform/pddf/platform-api-pddf-base.dep

diff --git a/platform/pddf/platform-api-pddf-base.dep b/platform/pddf/platform-api-pddf-base.dep
index 9184925b1..0adf0ff2d 100644
--- a/platform/pddf/platform-api-pddf-base.dep
+++ b/platform/pddf/platform-api-pddf-base.dep
@@ -7,3 +7,7 @@ DEP_FILES += $(shell git ls-files $(MPATH))
$(PDDF_PLATFORM_API_BASE_PY2)_CACHE_MODE := GIT_CONTENT_SHA
$(PDDF_PLATFORM_API_BASE_PY2)_DEP_FLAGS := $(SONIC_COMMON_FLAGS_LIST)
$(PDDF_PLATFORM_API_BASE_PY2)_DEP_FILES := $(DEP_FILES)
+
+$(PDDF_PLATFORM_API_BASE_PY3)_CACHE_MODE := GIT_CONTENT_SHA
+$(PDDF_PLATFORM_API_BASE_PY3)_DEP_FLAGS := $(SONIC_COMMON_FLAGS_LIST)
+$(PDDF_PLATFORM_API_BASE_PY3)_DEP_FILES := $(DEP_FILES)

git diff platform/pddf/platform-api-pddf-base.mk

diff --git a/platform/pddf/platform-api-pddf-base.mk b/platform/pddf/platform-api-pddf-base.mk
index 80478d4f3..6938199dd 100644
--- a/platform/pddf/platform-api-pddf-base.mk
+++ b/platform/pddf/platform-api-pddf-base.mk
@@ -13,3 +13,14 @@ SONIC_PYTHON_WHEELS += $(PDDF_PLATFORM_API_BASE_PY2)

export pddf_platform_api_base_py2_wheel_path="$(addprefix $(PYTHON_WHEELS_PATH)/,$(PDDF_PLATFORM_API_BASE_PY2))"
export PDDF_PLATFORM_API_BASE_PY2
+
+PDDF_PLATFORM_API_BASE_PY3 = sonic_platform_pddf_common-$(PDDF_PLATFORM_API_BASE_VERSION)-py3-none-any.whl
+$(PDDF_PLATFORM_API_BASE_PY3)_SRC_PATH = $(PLATFORM_PDDF_PATH)/platform-api-pddf-base
+$(PDDF_PLATFORM_API_BASE_PY3)_PYTHON_VERSION = 3
+$(PDDF_PLATFORM_API_BASE_PY3)_DEPENDS = $(SONIC_CONFIG_ENGINE)
+#$(PDDF_PLATFORM_API_BASE_PY3)_DEPENDS += $(PDDF_PLATFORM_API_BASE_PY2)
+$(PDDF_PLATFORM_API_BASE_PY3)_TEST = n

+SONIC_PYTHON_WHEELS += $(PDDF_PLATFORM_API_BASE_PY3)
+
+export pddf_platform_api_base_py3_wheel_path="$(addprefix $(PYTHON_WHEELS_PATH)/,$(PDDF_PLATFORM_API_BASE_PY3))"
+export PDDF_PLATFORM_API_BASE_PY3

git diff build_debian.sh

diff --git a/build_debian.sh b/build_debian.sh
index 6505a8e56..ba08965fd 100755
--- a/build_debian.sh
+++ b/build_debian.sh
@@ -281,6 +281,7 @@ sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y in
python-setuptools
python3-setuptools
python-jsonschema
*+ python3-jsonschema *
python-apt
traceroute
iputils-ping \

@jleveque
Copy link
Contributor Author

jleveque commented Dec 28, 2020

@fk410167: Thank you for testing and for your diffs! I have made the changes you suggested with one difference:

  • Rather than explicitly installing the python3-jsonschema Debian package, I removed the explicit installation of the Python 2 python-jsonschema package from both the host OS and the PMon container and instead added jsonschema=2.6.0 to the install_requires= list in setup.py. This way, the appropriate jsonschema package (Python 2 or Python 3) will get installed along with the PDDF package at install time via pip. Note that I pinned the version to 2.6.0 to match the latest version which is available via Debian. Version 3.2.0 is available via pip, but I wasn't sure if there would be any issues with PDDF and that version. You can feel free to test this out in your own time and update the version in setup.py in the future.

Please test and review again at your next convenience.

@FuzailBrcm
Copy link
Contributor

retest this please

@FuzailBrcm
Copy link
Contributor

@jleveque

Getting this error during the build.

12:25:29 /sonic/platform/pddf/platform-api-pddf-base /sonic
12:25:29 error in sonic-platform-pddf-common setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Invalid requirement, parse error at "'=2.6.0'"
12:25:29 [ FAIL LOG END ] [ target/python-wheels/sonic_platform_pddf_common-1.0-py2-none-any.whl ]
12:25:29 make: *** [slave.mk:596: target/python-wheels/sonic_platform_pddf_common-1.0-py2-none-any.whl] Error 1
12:25:30 DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.
12:26:47 make[1]: *** [Makefile.work:263: target/sonic-broadcom.bin] Error 2
12:26:47 make[1]: Leaving directory '/data/johnar/workspace/broadcom/buildimage-brcm-all-pr'
12:26:47 make: *** [Makefile:9: target/sonic-broadcom.bin] Error 2

@jleveque
Copy link
Contributor Author

@fk410167: Thanks for the heads-up. I omitted an =. Should be fixed now.

@FuzailBrcm
Copy link
Contributor

retest broadcom please

@FuzailBrcm
Copy link
Contributor

retest mellanox please

@jleveque jleveque marked this pull request as ready for review January 6, 2021 20:05
@jleveque
Copy link
Contributor Author

jleveque commented Jan 6, 2021

@fk410167: All check builds have passed. Can you please confirm the PR is good on your end?

@FuzailBrcm
Copy link
Contributor

@jleveque
Yes. Changes are good to go from my end.

@jleveque jleveque requested a review from lguohan January 7, 2021 06:43
@lguohan
Copy link
Collaborator

lguohan commented Jan 7, 2021

@fk410167, please sign off then?

Copy link
Contributor

@FuzailBrcm FuzailBrcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good to me

@jleveque jleveque merged commit e52581e into sonic-net:master Jan 7, 2021
@jleveque jleveque deleted the pddf_py3 branch January 7, 2021 18:03
yxieca pushed a commit that referenced this pull request Feb 23, 2021
- Make PDDF code compliant with both Python 2 and Python 3
- Align code with PEP8 standards using autopep8
- Build and install both Python 2 and Python 3 PDDF packages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants