Fix Python 3 'importlib' bug; Add support for Python 2 back in sonic-py-common#6933
Fix Python 3 'importlib' bug; Add support for Python 2 back in sonic-py-common#6933jleveque merged 4 commits intosonic-net:masterfrom jleveque:continue_support_py2_load_source
Conversation
…n 2 back for the time being
|
as my understanding, as part of the pr #6832, you only tested python3, but not python2, therefore python2 is broken? |
|
why is that the unit test determine-reboot-cause_test.py does not catch the issue? |
No. Python 3 was broken in the PMon container, where Python 2 is still installed, because it breaks the import mechanism when calling |
It appears to be environment-specific. I have not been able to determine the exact conditions necessary to produce the issue. The documentation shows examples of importing the machinery and util modules by their full paths, but does not explicitly state it should be done that way. @lguohan: I believe I should update this PR to do the same for all other instances to prevent any potential issues. What do you think? |
…py-common (#6933) Fix a strange bug introduced by #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.
…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.
…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.
Why I did it
Fix a strange bug introduced by #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:
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, callingimport importlib.machineryandimport importlib.utilcauses the proper package to be referenced, and themachineryandutilmodules 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 thatimport importlibis 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.
How I did it
import importlib.machineryandimport importlib.utilexplicitly_load_module_from_file()method. If Python 2, load source via theimpmodule, if Python 3, use theimportlibmodule.How to verify it
Ensure
ledddaemon starts properly with both Python 2 and Python 3Which release branch to backport (provide reason below if selected)