[Python] Migrate applications/scripts to import sonic-py-common package#5043
Merged
jleveque merged 23 commits intosonic-net:masterfrom Aug 3, 2020
jleveque:use_sonic-py-common
Merged
[Python] Migrate applications/scripts to import sonic-py-common package#5043jleveque merged 23 commits intosonic-net:masterfrom jleveque:use_sonic-py-common
jleveque merged 23 commits intosonic-net:masterfrom
jleveque:use_sonic-py-common
Conversation
11 tasks
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Collaborator
|
Build finished. No test results found. |
This comment has been minimized.
This comment has been minimized.
added 2 commits
July 30, 2020 19:59
This comment has been minimized.
This comment has been minimized.
yxieca
previously approved these changes
Jul 30, 2020
This was referenced Jul 30, 2020
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This pull request fixes 6 alerts when merging afd7e78 into 8dfc824 - view on LGTM.com fixed alerts:
|
lguohan
approved these changes
Aug 3, 2020
Collaborator
|
lgtm, one thing you might want to check is the arista platform, they have their platform code in their own directory, I do not know if you grep their directory or not. |
Contributor
Author
Yes. I grepped the entire repo as well as the submodules. Arista is not importing either sonic_device_util or sonic_daemon_base. |
jleveque
added a commit
that referenced
this pull request
Aug 3, 2020
Consolidate common SONiC Python-language functionality into one shared package (sonic-py-common) and eliminate duplicate code. The package currently includes four modules: - daemon_base - device_info - logger - task_base NOTE: This is a combination of all changes from #5003, #5049 and some changes from #5043 backported to align with the 201911 branch. As part of the 201911 port, I am not installing the Python 3 package in the base image or in the VS container, because we do not have pip3 installed, and we do not intend to migrate to Python 3 in 201911.
santhosh-kt
pushed a commit
to santhosh-kt/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…ge (sonic-net#5043) As part of consolidating all common Python-based functionality into the new sonic-py-common package, this pull request: 1. Redirects all Python applications/scripts in sonic-buildimage repo which previously imported sonic_device_util or sonic_daemon_base to instead import sonic-py-common, which was added in sonic-net#5003 2. Replaces all calls to `sonic_device_util.get_platform_info()` to instead call `sonic_py_common.get_platform()` and removes any calls to `sonic_device_util.get_machine_info()` which are no longer necessary (i.e., those which were only used to pass the results to `sonic_device_util.get_platform_info()`. 3. Removes unused imports to the now-deprecated sonic-daemon-base package and sonic_device_util.py module This is the next step toward resolving sonic-net#4999 Also reverted my previous change in which device_info.get_platform() would first try obtaining the platform ID string from Config DB and fall back to gathering it from machine.conf upon failure because this function is called by sonic-cfggen before the data is in the DB, in which case, the db_connect() call will hang indefinitely, which was not the behavior I expected. As of now, the function will always reference machine.conf.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
- Why I did it
As part of consolidating all common Python-based functionality into the new sonic-py-common package, this pull request:
sonic_device_util.get_platform_info()to instead callsonic_py_common.get_platform()and removes any calls tosonic_device_util.get_machine_info()which are no longer necessary (i.e., those which were only used to pass the results tosonic_device_util.get_platform_info().This is the next step toward resolving #4999
Also reverted my previous change in which device_info.get_platform() would first try obtaining the platform ID string from Config DB and fall back to gathering it from machine.conf upon failure because this function is called by sonic-cfggen before the data is in the DB, in which case, the db_connect() call will hang indefinitely, which was not the behavior I expected. As of now, the function will always reference machine.conf.
- How to verify it
Test that all affected Python applications/scripts continue to function properly
- Which release branch to backport (provide reason below if seleted)