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

[201911] Fix issues with sonic-py-common package #5191

Merged
merged 1 commit into from
Aug 15, 2020

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Aug 15, 2020

What I did:
Two fixes/change were made with new sonic-py-common package:

a) we should use get_platform() with new sonic_py-common package
instead of get_platform_info()

b) In 201911 DB Connector is still using db id based constructor
as following PR##4549
is not cherry-picked yet.
So revert the change to use db is instead of db_name for now. This will be updated when we move to db name

How I verified:
Because of (b) PMON process were bailing out. After fix things are fine.

a) we should use get_platform() with new sonic_py-common package
b) In 201911 DB Connector is still using db id based constructor
as following PR sonic-net#4549
is not cherry-picked yet. So revert the change to use db is insatead of
db_name for now.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi abdosi requested a review from jleveque August 15, 2020 01:02
@abdosi abdosi merged commit ef4d858 into sonic-net:201911 Aug 15, 2020
@abdosi abdosi deleted the platform-fixes branch August 15, 2020 01:10
@lguohan
Copy link
Collaborator

lguohan commented Aug 15, 2020

what kind of test can catch this?

@jleveque
Copy link
Contributor

jleveque commented Aug 15, 2020

As the final step of the transition to sonic-py-common, I indend to add unit tests (see: #4999) which can be run at package build time. Whenever anyone adds new functions like these to the library, we should ensure 100% test coverage before merging the PR.

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