Introduce sonic-py-common package #5003

jleveque · 2020-07-18T23:54:02Z

- Why I did it
To consolidate common SONiC Python-language functionality into one shared package and eliminate duplicate code. This is the first step toward resolving #4999

The package currently includes three modules:

daemon_base
device_info
logger

- How I did it
Add a new Python package, "sonic-py-common". This will replace the sonic-daemon-base package as well as the sonic_device_util.py file in sonic-config-engine.

I am building both Python 2 and Python 3 versions of the package. The Python 2 package is for backward-compatibility with existing Python 2 code, and the Python 3 version is for use in the SNMP container as well as to prepare for the eventual migration of all Python code to Python 3, since Python 2 is no longer supported as of January 1, 2020.

I think that db_connect() and load_platform_util() should also be moved out of daemon_base.py and into a separate module, but I opted against it for this PR, just to make the initial transition from sonic-daemon-base to sonic-py-common more straightforward. This can be taken care of in a future refactor.

- How to verify it
Test using the following script (replace python2 with python3 in shebang to test Python 3 package):

#!/usr/bin/env python2

from sonic_py_common import device_info, logger, daemon_base

print("machine_info: {}".format(device_info.get_machine_info()))
print("asic_cfg_path: {}".format(device_info.get_asic_conf_file_path()))
print("platform: {}".format(device_info.get_platform()))
print("hwsku: {}".format(device_info.get_hwsku()))

plat, sku = device_info.get_platform_and_hwsku()
print("plat: {}, sku: {}".format(plat, sku))

plat_path, sku_path = device_info.get_paths_to_platform_and_hwsku_dirs()
print("plat_path: {}, sku_path: {}".format(plat_path, sku_path))

print("port_config_path: {}".format(device_info.get_path_to_port_config_file()))
print("version_info: {}".format(device_info.get_sonic_version_info()))
print("num_npus: {}".format(device_info.get_num_npus()))
print("is_multi_npu: {}".format(device_info.is_multi_npu()))
print("namespaces: {}".format(device_info.get_namespaces()))
print("all_namespaces: {}".format(device_info.get_all_namespaces()))

my_logger = logger.Logger("test")
my_logger.log_error("Test error message; Minimum log priority = PRIORITY_NOTICE")
my_logger.log_warning("Test warning message; Minimum log priority = PRIORITY_NOTICE")
my_logger.log_notice("Test notice message; Minimum log priority = PRIORITY_NOTICE")
# The following SHOULDN'T get logged
my_logger.log_info("Test info message; Minimum log priority = PRIORITY_NOTICE")
my_logger.log_debug("Test debug message; Minimum log priority = PRIORITY_NOTICE")

my_logger.set_min_log_priority_info()
my_logger.log_error("Test error message; Minimum log priority = PRIORITY_INFO")
my_logger.log_warning("Test warning message; Minimum log priority = PRIORITY_INFO")
my_logger.log_notice("Test notice message; Minimum log priority = PRIORITY_INFO")
my_logger.log_info("Test info message; Minimum log priority = PRIORITY_INFO")
# The following SHOULDN'T get logged
my_logger.log_debug("Test debug message; Minimum log priority = PRIORITY_INFO")

my_logger.set_min_log_priority_debug()
my_logger.log_error("Test error message; Minimum log priority = PRIORITY_DEBUG")
my_logger.log_warning("Test warning message; Minimum log priority = PRIORITY_DEBUG")
my_logger.log_notice("Test notice message; Minimum log priority = PRIORITY_DEBUG")
my_logger.log_info("Test info message; Minimum log priority = PRIORITY_DEBUG")
my_logger.log_debug("Test debug message; Minimum log priority = PRIORITY_DEBUG")


class MyDaemon(daemon_base.DaemonBase):
    def run(self):
        self.log_error("Test error message; Minimum log priority = PRIORITY_NOTICE")
        self.log_warning("Test warning message; Minimum log priority = PRIORITY_NOTICE")
        self.log_notice("Test notice message; Minimum log priority = PRIORITY_NOTICE")
        # The following SHOULDN'T get logged
        self.log_info("Test info message; Minimum log priority = PRIORITY_NOTICE")
        self.log_debug("Test debug message; Minimum log priority = PRIORITY_NOTICE")

        self.set_min_log_priority_info()
        self.log_error("Test error message; Minimum log priority = PRIORITY_INFO")
        self.log_warning("Test warning message; Minimum log priority = PRIORITY_INFO")
        self.log_notice("Test notice message; Minimum log priority = PRIORITY_INFO")
        self.log_info("Test info message; Minimum log priority = PRIORITY_INFO")
        # The following SHOULDN'T get logged
        self.log_debug("Test debug message; Minimum log priority = PRIORITY_INFO")

        self.set_min_log_priority_debug()
        self.log_error("Test error message; Minimum log priority = PRIORITY_DEBUG")
        self.log_warning("Test warning message; Minimum log priority = PRIORITY_DEBUG")
        self.log_notice("Test notice message; Minimum log priority = PRIORITY_DEBUG")
        self.log_info("Test info message; Minimum log priority = PRIORITY_DEBUG")
        self.log_debug("Test debug message; Minimum log priority = PRIORITY_DEBUG")

my_daemon = MyDaemon("test_daemon")
my_daemon.run()

- Description for the changelog
Consolidate common SONiC Python-language functionality into one shared package (sonic-py-common) and eliminate duplicate code.

- Next Steps

Migrate all Python applications in sonic-buildimage from sonic-daemon-base and sonic_device_util to sonic-py-common
Migrate all Python applications in submodules from sonic-daemon-base and sonic_device_util to sonic-py-common
Remove sonic-daemon-base and sonic_device_util entirely
Migrate common code from sonic-utilities into sonic-py-common
Add regression tests for sonic--py-common to sonic-mgmt repo

keboliu · 2020-07-22T02:26:38Z

src/sonic-py-common/sonic_py_common/logger.py

+Logging functionality for SONiC Python applications
+"""
+
+FACILITY_USER = syslog.LOG_USER


IMO there is an issue in current logger implementation, there is no log level control, by default all the debug log will be printed out, if we can have some way to control the log level it would be great.

Thanks for bringing attention to this. I can add that to this PR fairly easily. What should we set as the default minimum log level -- NOTICE? INFO?

I added a new commit which adds a new method, set_min_log_priority(), and sets the default minimum priority to NOTICE. We can change the default to INFO if there's a consensus to change it.

This is Great Joe.

zhenggen-xu

As this will become the common functions, I think we should have automation test cases (probably vs tests) to cover them. Should we consider to add something similar to the tests cases in your PR description to be automated?

zhenggen-xu · 2020-07-25T05:55:36Z

src/sonic-py-common/sonic_py_common/logger.py

+ def log(self, priority, msg, also_print_to_console=False):
+ self.syslog.syslog(priority, msg)
+
+ if also_print_to_console:


Should we check log level (priority) here too?

Good question. I think it's debatable. In the existing implementation of the Logger class, we do not. So I say for now we keep it as-is. If we decide we want to at a later time, we can add the checks.

jleveque · 2020-07-26T20:42:58Z

As this will become the common functions, I think we should have automation test cases (probably vs tests) to cover them. Should we consider to add something similar to the tests cases in your PR description to be automated?

Absolutely. I have now explicitly added this to the "Next Steps" in the description as well as in issue #4999

lguohan

jleveque · 2020-07-28T16:41:31Z

Removed "Request for 201911 Branch" label, as backporting this change and the related changes will most likely be better performed by creating new PRs against the 201911 branch.

…ge (#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 #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 #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.

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.

jleveque added Enhancement ➕ Request for 202006 Branch labels Jul 18, 2020

jleveque requested review from lguohan, yxieca, keboliu, SuvarnaMeenakshi, arlakshm and abdosi July 18, 2020 23:54

jleveque self-assigned this Jul 18, 2020

sonic-net deleted a comment from lgtm-com bot Jul 19, 2020

jleveque requested review from vdahiya12 and sujinmkang July 19, 2020 00:54

jleveque mentioned this pull request Jul 20, 2020

Common functions for show CLI support on multi ASIC sonic-net/sonic-utilities#999

Merged

jleveque added 11 commits July 20, 2020 22:37

Introduce sonic-py-common package

5f74856

Install sonic_py_common in PMon container

ac83ce3

Add sonic-py-common as dependency of mlnx-platform-api

55a1d79

Add sonic-py-common as dependency of sonic-thermalctld

862e786

Fix LGTM alerts

284c0db

Update docstring for get_asic_conf_file_path()

1de1385

Update comment

97fef08

Docstring style

bedbc1a

Remove BGP cache file that snuck in to a commit

f49c4da

Add TODO comment above moving multi-ASIC functionality to its own module

d11f1a6

Revert changes to celestica helper.py file; Will save for a separate PR

40a450b

yxieca previously approved these changes Jul 21, 2020

View reviewed changes

jleveque mentioned this pull request Jul 21, 2020

Create a Python library to house common general SONiC-related APIs #4999

Open

11 tasks

keboliu reviewed Jul 22, 2020

View reviewed changes

Add ability to control minimum log priority, set default to NOTICE

d65f683

jleveque dismissed yxieca’s stale review via d65f683 July 22, 2020 03:45

Fix LGTM alert

4288a8b

jleveque mentioned this pull request Jul 22, 2020

Common functions for Multi ASIC CLIs #4973

Merged

jleveque added 8 commits July 22, 2020 21:14

Build Python 3 verison of package, install in host OS and SNMP container

50e7b49

Install Python 3 package using pip3

c1e84de

Make exception statements Python3-compliant

a3d280e

Add natsort as dependency

f2d0fb0

Separate standard imports from third-party imports with a blank line

f1a541f

Add pyyaml as dependency

59f56d1

Add 'LOG_' prefix to facilities and priorities

461a56c

Install python3-setuptools in build_debian.sh

95419cd

jleveque mentioned this pull request Jul 25, 2020

[platform] Add Support For Environment Variable File #5010

Merged

Add missing 'LOG_' prefix

3365c7b

zhenggen-xu reviewed Jul 25, 2020

View reviewed changes

jleveque mentioned this pull request Jul 25, 2020

Platform monitor changes in daemon_base for multi_asic #4932

Merged

jleveque requested a review from judyjoseph July 25, 2020 22:45

jleveque added the Request for 201911 Branch label Jul 26, 2020

yxieca approved these changes Jul 27, 2020

View reviewed changes

lguohan approved these changes Jul 27, 2020

View reviewed changes

jleveque merged commit c0d1616 into sonic-net:master Jul 27, 2020

jleveque deleted the add_sonic-py-common branch July 27, 2020 06:15

jleveque mentioned this pull request Jul 27, 2020

[Python] Migrate applications/scripts to import sonic-py-common package sonic-net/sonic-utilities#1008

Merged

jleveque removed the Request for 201911 Branch label Jul 28, 2020

This was referenced Jul 28, 2020

[201911] Introduce sonic-py-common package #5063

Merged

[Python] Migrate applications/scripts to import sonic-py-common package #5043

Merged

jleveque added the sonic-py-common label Aug 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce sonic-py-common package #5003

Introduce sonic-py-common package #5003

jleveque commented Jul 18, 2020 •

edited

Loading

keboliu Jul 22, 2020

jleveque Jul 22, 2020 •

edited

Loading

jleveque Jul 22, 2020 •

edited

Loading

keboliu Jul 30, 2020

zhenggen-xu left a comment

zhenggen-xu Jul 25, 2020

jleveque Jul 26, 2020

jleveque commented Jul 26, 2020

lguohan left a comment

jleveque commented Jul 28, 2020

Introduce sonic-py-common package #5003

Introduce sonic-py-common package #5003

Conversation

jleveque commented Jul 18, 2020 • edited Loading

keboliu Jul 22, 2020

Choose a reason for hiding this comment

jleveque Jul 22, 2020 • edited Loading

Choose a reason for hiding this comment

jleveque Jul 22, 2020 • edited Loading

Choose a reason for hiding this comment

keboliu Jul 30, 2020

Choose a reason for hiding this comment

zhenggen-xu left a comment

Choose a reason for hiding this comment

zhenggen-xu Jul 25, 2020

Choose a reason for hiding this comment

jleveque Jul 26, 2020

Choose a reason for hiding this comment

jleveque commented Jul 26, 2020

lguohan left a comment

Choose a reason for hiding this comment

jleveque commented Jul 28, 2020

jleveque commented Jul 18, 2020 •

edited

Loading

jleveque Jul 22, 2020 •

edited

Loading

jleveque Jul 22, 2020 •

edited

Loading