Skip to content

Commit

Permalink
Fix Python 3 'importlib' bug; Add support for Python 2 back in sonic-…
Browse files Browse the repository at this point in the history
…py-common (sonic-net#6933)

Fix a strange bug introduced by sonic-net#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.
  • Loading branch information
jleveque authored and yxieca committed Mar 4, 2021
1 parent 12cc180 commit 2e1e7e0
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 8 deletions.
3 changes: 2 additions & 1 deletion src/sonic-ctrmgrd/tests/common_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import copy
import importlib
import importlib.machinery
import importlib.util
import json
import os
import subprocess
Expand Down
3 changes: 2 additions & 1 deletion src/sonic-host-services/tests/determine-reboot-cause_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import importlib
import importlib.machinery
import importlib.util
import sys
import os
import pytest
Expand Down
3 changes: 2 additions & 1 deletion src/sonic-host-services/tests/procdockerstatsd_test.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import importlib
import importlib.machinery
import importlib.util
import sys
import os
import pytest
Expand Down
23 changes: 18 additions & 5 deletions src/sonic-py-common/sonic_py_common/daemon_base.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import importlib
import signal
import sys

Expand Down Expand Up @@ -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


Expand Down

0 comments on commit 2e1e7e0

Please sign in to comment.