Force importing yaml from the system dist-packages #2448
Conversation
| "missing-yaml-module", | ||
| """\ | ||
| Couldn't import the YAML module from /usr/lib/python3/dist-packages. | ||
| Make sure the 'python3-yaml' package is installed correctly.""", |
There was a problem hiding this comment.
Would it be unnecessary to add the command to install the python3-yaml package i.e. sudo apt install python3-yaml?
There was a problem hiding this comment.
Yes unecessary - it is a dependency of ubuntu-advantage-tools - if this is not installed correctly then they messed it up, and a simple apt install may not be sufficient...
|
@orndorffgrant hmmm well I'll investigate and see why it does not break... Thanks for the heads up |
renanrodrigo
force-pushed
the
yaml-very-safe-load
branch
2 times, most recently
from
873418d
to
4d786ae
Compare
|
@orndorffgrant surprisingly, Python itself ensures retro compatibility and it does not fail until 3.10! |
Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
Check for the existence of yaml in the system libs before going the Return an error when no module is found. Using the system yaml guarantees that safe_load and safe_dump works. LP: #2007234 LP: #2007241 Signed-off-by: Renan Rodrigo <renanrodrigo@canonical.com>
renanrodrigo
force-pushed
the
yaml-very-safe-load
branch
from
4d786ae
to
6b27726
Compare
|
CI failures unrelated |
When users install a new pyyaml using pip, it will end up in pythonpath with a higher priority.
This PR introduces some logic to import yaml only directly from the system dist-packages.
This fixes two bugs:
LP: #2007234 : If yaml is not present, then a UserFacingError is thrown - this can only happen if the
python3-yamldeb is not present or was directly modified, and as it is a dependency, it does not make sense to support anything like this.LP: #2007241 : The system yaml won't have a version which is incompatible with the python version, so the safe_load bug is prevented.
Test Steps
Added unit tests and integration tests to verify the fix.
Checklist
Does this PR require extra reviews?