-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[Python] Migrate applications/scripts to import sonic-py-common package #5043
Conversation
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.
Build finished. No test results found. |
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.
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:
|
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. |
Yes. I grepped the entire repo as well as the submodules. Arista is not importing either sonic_device_util or sonic_daemon_base. |
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.
…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.
- 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)