From 980a024dd4216737360e8d783a470fdef65ce3be Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Tue, 2 Mar 2021 18:31:19 -0800 Subject: [PATCH] Fix Python 3 'importlib' bug; Add support for Python 2 back in sonic-py-common (#6933) Fix a strange bug introduced by https://github.com/Azure/sonic-buildimage/pull/6832 which would only occur in environments with both Python 2 and Python 3 installed (e.g., the PMon container). Error messages such as the following would be seen: ``` ERR pmon#ledd[29]: Failed to load ledutil: module 'importlib' has no attribute 'machinery' ``` This is very odd, and it seems like the Python 2 version of importlib, which is basically just a stub, is taking precedence over the Python 3 version. I found that this occurs when calling `import importlib`. However, calling `import importlib.machinery` and `import importlib.util` causes the proper package to be referenced, and the `machinery` and `util` modules are loaded successfully. This is how it is specified in examples in the official documentation, however there is nothing mentioned regarding that it *should* be done this way or that `import importlib` is unreliable. Also, since sonic-py-common is still used in environments with Python 2 installed we should maintain support for both Python 2 and 3 until we completely deprecate Python 2, so I have added this back in. --- src/sonic-ctrmgrd/tests/common_test.py | 3 ++- .../tests/determine-reboot-cause_test.py | 3 ++- .../tests/procdockerstatsd_test.py | 3 ++- .../sonic_py_common/daemon_base.py | 23 +++++++++++++++---- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/sonic-ctrmgrd/tests/common_test.py b/src/sonic-ctrmgrd/tests/common_test.py index 35447be7ad..5b3eae151c 100755 --- a/src/sonic-ctrmgrd/tests/common_test.py +++ b/src/sonic-ctrmgrd/tests/common_test.py @@ -1,5 +1,6 @@ import copy -import importlib +import importlib.machinery +import importlib.util import json import os import subprocess diff --git a/src/sonic-host-services/tests/determine-reboot-cause_test.py b/src/sonic-host-services/tests/determine-reboot-cause_test.py index 924befdf48..381313ba97 100644 --- a/src/sonic-host-services/tests/determine-reboot-cause_test.py +++ b/src/sonic-host-services/tests/determine-reboot-cause_test.py @@ -1,4 +1,5 @@ -import importlib +import importlib.machinery +import importlib.util import sys import os import pytest diff --git a/src/sonic-host-services/tests/procdockerstatsd_test.py b/src/sonic-host-services/tests/procdockerstatsd_test.py index db7c6a1f0e..bb218e52ce 100644 --- a/src/sonic-host-services/tests/procdockerstatsd_test.py +++ b/src/sonic-host-services/tests/procdockerstatsd_test.py @@ -1,4 +1,5 @@ -import importlib +import importlib.machinery +import importlib.util import sys import os import pytest diff --git a/src/sonic-py-common/sonic_py_common/daemon_base.py b/src/sonic-py-common/sonic_py_common/daemon_base.py index 3797c19b6b..745b3c55ba 100644 --- a/src/sonic-py-common/sonic_py_common/daemon_base.py +++ b/src/sonic-py-common/sonic_py_common/daemon_base.py @@ -1,4 +1,3 @@ -import importlib import signal import sys @@ -26,12 +25,26 @@ def db_connect(db_name, namespace=EMPTY_NAMESPACE): return swsscommon.DBConnector(db_name, REDIS_TIMEOUT_MSECS, True, namespace) +# TODO: Consider moving this logic out of daemon_base and into antoher file +# so that it can be used by non-daemons. We can simply call that function here +# to retain backward compatibility. def _load_module_from_file(module_name, file_path): - loader = importlib.machinery.SourceFileLoader(module_name, file_path) - spec = importlib.util.spec_from_loader(loader.name, loader) - module = importlib.util.module_from_spec(spec) - loader.exec_module(module) + module = None + + # TODO: Remove this check once we no longer support Python 2 + if sys.version_info.major == 3: + import importlib.machinery + import importlib.util + loader = importlib.machinery.SourceFileLoader(module_name, file_path) + spec = importlib.util.spec_from_loader(loader.name, loader) + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + else: + import imp + module = imp.load_source(module_name, file_path) + sys.modules[module_name] = module + return module