Use looseversion in place of distutils#904
Use looseversion in place of distutils#904freyes wants to merge 1 commit intojuju:stable/caracalfrom
Conversation
(cherry picked from commit 012b2d6)
|
Blocker for s390x validation |
|
|
||
| pbr!=2.1.0,>=2.0.0 # Apache-2.0 | ||
|
|
||
| looseversion;python_version >= '3.12' |
There was a problem hiding this comment.
how does this work when being used in a classic charm?
There was a problem hiding this comment.
Based on your comment above, it probably breaks unless the parent charm ensures that the package is installed.
| try: | ||
| from distutils.version import LooseVersion | ||
| except ImportError: | ||
| from looseversion import LooseVersion |
There was a problem hiding this comment.
shouldn't this be more like:
try:
from distutils.version import LooseVersion
except ImportError:
try:
# reactive charm path, where the venv already has this package
from looseversion import LooseVersion
except ImportError:
# classic charm code path where python packages are installed via apt-get
apt_install(['python3-looseversion'], fatal=True)
from looseversion import LooseVersionThere was a problem hiding this comment.
I really don't like this pattern where charmhelpers runs apt_install; I'd much rather the parent charm resolved the dependencies and installed python3-looseversion where necessary; (which might be all of them in the future). It makes unit testing harder when apt_install() is at module load time.
| try: | ||
| from distutils.version import LooseVersion | ||
| except ImportError: | ||
| from looseversion import LooseVersion |
There was a problem hiding this comment.
I really don't like this pattern where charmhelpers runs apt_install; I'd much rather the parent charm resolved the dependencies and installed python3-looseversion where necessary; (which might be all of them in the future). It makes unit testing harder when apt_install() is at module load time.
|
|
||
| pbr!=2.1.0,>=2.0.0 # Apache-2.0 | ||
|
|
||
| looseversion;python_version >= '3.12' |
There was a problem hiding this comment.
Based on your comment above, it probably breaks unless the parent charm ensures that the package is installed.
|
Holding off merging whilst @freyes conversation is concluded; there may be further changes needed to the original commits. |
(cherry picked from commit 012b2d6)