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

[Platform][nokia]: python3-smbus package add with python3 and jinja fixes #6416

Merged
merged 3 commits into from
Jan 12, 2021

Conversation

carl-nokia
Copy link
Contributor

@carl-nokia carl-nokia commented Jan 11, 2021

- Why I did it
fix platform driver breakage due to python3 upgrade and fix load minigraph errors with config load_minigraph -y

- How I did it
added python3-smbus to the pmon docker template since the previous was python2 specific
fixed additional "ord" python2 specific code
fixed the jinja templates used by qos reload - the template logic required data to be parsed

- How to verify it
run "show platform XXX" commands and verify output
run "sudo config load_minigraph -y" and verify configuration
run "show interfaces XXX" and verify output

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

  • 201811
  • 201911
  • 202006
  • 202012

- Description for the changelog

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

@lguohan lguohan requested a review from jleveque January 11, 2021 20:43
Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Please fill out the PR description field

@carl-nokia
Copy link
Contributor Author

No problem - I see what you are saying - I changed the order

@@ -21,6 +21,7 @@ RUN apt-get update && \
librrd-dev \
rrdtool \
python-smbus \
python3-smbus \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed by the Nokia sonic-platform package? If so , I would prefer that you add it in the install_requires list of your setup.py file. This way it will get installed along with your package, and will not be installed if not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is needed by the Nokia sonic-platform package and we see Arista using it also btw. I attempted adding to install_requires list of setup.py file as you suggested, but it appears that mechanism is at runtime install time and did not work. See error below:
WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.HTTPSConnection object at 0xb5731730>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/python3-smbus/
Build time install solution is preferred I think - putting the package in our debian another way - still working on alternatives other than the current Arista method multiple debians

Copy link
Contributor

Choose a reason for hiding this comment

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

@carl-nokia: Ah yes. The vendor's sonic-platform package gets installed in the PMon container at runtime, thus you are correct, my suggestion will not work. I am OK with this solution for now. Sorry to trouble you.

@lguohan lguohan merged commit 380edf0 into sonic-net:master Jan 12, 2021
lguohan pushed a commit that referenced this pull request Jan 15, 2021
…ixes (#6416)

fix platform driver breakage due to python3 upgrade and fix load minigraph errors with config load_minigraph -y

**- How I did it**
added python3-smbus to the pmon docker template since the previous was python2 specific 
fixed additional "ord" python2 specific code 
fixed the jinja templates used by qos reload - the template logic required data to be parsed 

**- How to verify it**
run "show platform XXX" commands and verify output
run "sudo config load_minigraph -y" and verify configuration 
run "show interfaces XXX" and verify output 

Co-authored-by: Carl Keene <keene@nokia.com>
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.

3 participants