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

[Nvidia] [202205] Update syncd docker to use python version 3 #17733

Closed

Conversation

liorghub
Copy link
Contributor

@liorghub liorghub commented Jan 10, 2024

Why I did it

Python 2 is obsolete and no longer supported.

Work item tracking
  • Microsoft ADO (number only):

How I did it

Change several packages installation commands in Nvidia syncd dockerfile to Python3 packages.
Change was done for regular image and for RPC image as well.

How to verify it

I tested several SDK API scripts manually (while all others are tested by SDK verification) and ran regression tests that run on RPC version.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

@liorghub liorghub requested a review from lguohan as a code owner January 10, 2024 06:34
@liorghub liorghub changed the title [Nvidia] Update syncd docker to use python version 3 [Nvidia] [202205] Update syncd docker to use python version 3 Jan 10, 2024
@dprital
Copy link
Collaborator

dprital commented Jan 25, 2024

@yxieca , Can you please approve and merge ?

@yxieca
Copy link
Contributor

yxieca commented Jan 25, 2024

@yxieca , Can you please approve and merge ?

@dprital can you elaborate why do we need to make this change when 202205 qualification is at the tail end? Are these commands broken in pyhon2? I fully support commands become python3 compliant or python3 only in the newer branches.

@liorghub
Copy link
Contributor Author

@yxieca The initial plan was to remove python2 from Nvidia syncd container, but then I saw that Python2 exists in RPC container therefore I handled RPC container as well.
If you think it is not the right approach, may I create a different PR that contains only the changes in Nvidia syncd container?

@dprital
Copy link
Collaborator

dprital commented Jan 28, 2024

@yxieca , Python2 is deprecated from NVIDIA (Mellanox) syncd so it was removed. This is the reason for this PR.

@yxieca
Copy link
Contributor

yxieca commented Jan 29, 2024

@yxieca , Python2 is deprecated from NVIDIA (Mellanox) syncd so it was removed. This is the reason for this PR.

@liorghub @dprital I fully support removing python2 code. However, I don't think we should do it in 202205 at this point when qualification activity is done or almost done. This change can go to newer branches.

@dprital
Copy link
Collaborator

dprital commented Jan 29, 2024

@yxieca , Python2 is deprecated from NVIDIA (Mellanox) syncd so it was removed. This is the reason for this PR.

@liorghub @dprital I fully support removing python2 code. However, I don't think we should do it in 202205 at this point when qualification activity is done or almost done. This change can go to newer branches.

@yxieca , OK. This is the PR for Master (and request to cherry pick to 202305 and 202311). Can you please Approve and merge ?

#17735

@liorghub liorghub closed this Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants