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

[sonic-cfggen] fix name conflict between sonic_platform package and sonic_platform.py #2875

Merged
merged 5 commits into from
May 15, 2019
Merged

[sonic-cfggen] fix name conflict between sonic_platform package and sonic_platform.py #2875

merged 5 commits into from
May 15, 2019

Conversation

keboliu
Copy link
Collaborator

@keboliu keboliu commented May 8, 2019

- What I did
There is a name conflict between a module "sonic_platform.py" which sonic-cfggen is using and the new added "sonic_platform" package form each platform's implementation of new platform API.

This name conflict cause pmon container not able to go up since sonic-cfggen can't load correct "sonic_platform.py". So rename "sonic_platform.py" to "sonic_device_util.py" as a temporary mitigation to this issue. In the long run, this "sonic_platform.py" will be moved out and shared with multiple applications.

- How I did it
rename "sonic_platform.py" to "sonic_device_util.py" and change sonic-cfggen, package setup.py accordingly.

- How to verify it

run sonic-cfggen

- Description for the changelog

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

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.

@keboliu: Thanks for this fix. However, please note that there are scripts in sonic-utilities which also reference this file. They will also require updates: https://github.com/Azure/sonic-utilities/search?q=sonic_platform&unscoped_q=sonic_platform

Can you please check the other submodules as well?

I will wait to merge this until we have all PRs ready for merge so that we don't break anything.

@keboliu
Copy link
Collaborator Author

keboliu commented May 9, 2019

@jleveque thanks for the heads-up, I created another PR for sonic-utilities repo sonic-net/sonic-utilities#528. I searched other repos and they should be clean.

@jleveque
Copy link
Contributor

@keboliu: I opted against updating the submodule in this PR as it makes it harder to cherry-pick commits into other branches.

@keboliu
Copy link
Collaborator Author

keboliu commented May 10, 2019

@jleveque I realized that and just reverted.

@jleveque
Copy link
Contributor

jleveque commented May 15, 2019

@keboliu: After discussion with Guohan, we think it is best to continue the old practice of adding the submodule update into any PRs which require it to build sucessfully, even if it makes the cherry-pick more complicated. Can you please update the sonic-utilities submodule here? I will then merge this after it passes the PR checks.

@keboliu
Copy link
Collaborator Author

keboliu commented May 15, 2019

@jleveque sure.

@jleveque jleveque changed the title [sonic-cfggen] fix name conflict between sonic_platfrom package and sonic_platform.py [sonic-cfggen] fix name conflict between sonic_platform package and sonic_platform.py May 15, 2019
@jleveque
Copy link
Contributor

Retest this please.

@keboliu
Copy link
Collaborator Author

keboliu commented May 15, 2019

@jleveque the Broadcom image build failure should irrelevant.

@lguohan lguohan merged commit aa90cae into sonic-net:master May 15, 2019
@keboliu keboliu deleted the fix-name-confilict branch May 24, 2019 06:59
MichelMoriniaux pushed a commit to criteo-forks/sonic-buildimage that referenced this pull request May 28, 2019
…onic_platform.py (sonic-net#2875)

* fix name conflict between sonic_platfrom package and sonic_platform.py

* update sonic-utility submodule to pickup lastest fix

* Revert "update sonic-utility submodule to pickup lastest fix"

This reverts commit f66aa99.

* update sonic-utility sub module
yxieca pushed a commit that referenced this pull request Jul 16, 2019
…onic_platform.py (#2875)

* fix name conflict between sonic_platfrom package and sonic_platform.py

* update sonic-utility submodule to pickup lastest fix

* Revert "update sonic-utility submodule to pickup lastest fix"

This reverts commit f66aa99.

* update sonic-utility sub module
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.

7 participants