Align the python-libjuju dependency with the installed juju version #677
Conversation
|
@hemanthnakkina , what do you think about this patch?, it's basically setting the python-libjuju dependency dynamically based on the juju client version available in the system. |
| if int(version[0]) >= 3: | ||
| juju_ver_n = version[0:3] | ||
| juju_ver_n1 = "%s.%s" % (version[0], int(version[2]) + 1) | ||
| install_require.append('juju>=%s,<%s' % (juju_ver_n, |
There was a problem hiding this comment.
I am sceptical about using juju>=juju_ver_n especially for cases of any new juju major series release (using terminology mentioned in [1]) like 3.x -> 3.y or 4.x -> 4,y
Since there is usually couple of days gap between Juju release and py-libjuju release.
Here is the past data for 3.6 release for juju and pylibjuju [2] [3]
We need to check with juju team if they really follows what they mentioned in documentation [1] to use juju>=version[0],<juju_ver_n1
[1] https://canonical-juju.readthedocs-hosted.com/en/latest/user/reference/juju/juju-cross-version-compatibility/#juju-cli-controllers-and-ref-agents
[2] https://github.com/juju/juju/releases/tag/v3.6.0
[3] https://github.com/juju/python-libjuju/releases/tag/3.6.0.0
There was a problem hiding this comment.
I'd expect libjuju to significantly lag behind juju for new major versions. But then, running e.g. libjuju 3.x with juju 4.x is likely to run into compatibility issues, so I'm thinking failing the run for major version mismatches might be a good decision anyway.
Wonder if the version parsing should be made more robust to better handle version string format changes or double-digit versions though.
There was a problem hiding this comment.
I pushed a new revision of that patch with a more reasonable parsing.
|
On Wed, 2025-03-05 at 09:37 -0800, Peter Sabaini wrote:
@sabaini commented on this pull request.
In setup.py [1]:
> @@ -50,7 +51,19 @@
if os.environ.get("TEST_JUJU3"):
install_require.append('juju')
else:
- install_require.append('juju<3.0.0')
+ try:
+ version = subprocess.check_output(['juju', '--version'],
+ universal_newlines=True).strip()
+ print('juju --version ->', version)
+ if int(version[0]) >= 3:
+ juju_ver_n = version[0:3]
+ juju_ver_n1 = "%s.%s" % (version[0], int(version[2]) + 1)
+ install_require.append('juju>=%s,<%s' % (juju_ver_n,
I'd expect libjuju to significantly lag behind juju for new major versions. But then, running e.g.
libjuju 3.x with juju 4.x is likely to run into compatibility issues, so I'm thinking failing the
run for major version mismatches might be a good decision anyway.
Wonder if the version parsing should be made more robust to better handle version string format
changes or double-digit versions though.
mmmmh, yeah, for future proof, but I see unlikely to see a "3.10" juju version though
… —
Reply to this email directly, view it on GitHub [1], or unsubscribe [2].
You are receiving this because you authored the thread.Message ID: <openstack-
***@***.***>
--
Felipe Reyes
Software Engineer @ Canonical
***@***.*** (GPG:0x9B1FFF39)
Launchpad: ~freyes | IRC: freyes
[1] setup.py #677 (comment)
[2] unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAYLH3PTDDP3GPJVPDBXST2S4Y43AVCNFSM6AAAAABYKJNJIKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNRRHA4TCNBVGE
|
| universal_newlines=True).strip() | ||
| print('juju --version ->', version) | ||
| if int(version[0]) >= 3: | ||
| (major, minor) = version.split('.')[0:2] |
There was a problem hiding this comment.
This will fail whenever minor version cropped into double digit like 4.10
(major, minor, *_) = version.split('.')
juju_ver_n = "%s.%s" % (major, minor)
juju_ver_n1 = "%s.%s" % (major, int(minor) + 1)
There was a problem hiding this comment.
I don't think that will make that piece of code fail:
>>> version = "4.10.6-genericlinux-amd64"
>>> (major, minor) = version.split('.')[0:2]
>>> major
'4'
>>> minor
'10'
I did fix this line though -> https://github.com/openstack-charmers/zaza/compare/56c4edb7dee667ed7169f2ae4ed5688ec59afb4f..cb31096758c3f56aac13b06dafe93b3e942cd949
There was a problem hiding this comment.
AAh such a silly mistake from me, you are right
There was a problem hiding this comment.
missing '' for . in the new change
python-libjuju is released with a tight dependency with the underlying juju version, so when using juju-3.4 , python-libjuju-3.4 is needed, any mismatch will result in failures later. This patch will still honor the semantic of TEST_JUJU3.
| universal_newlines=True).strip() | ||
| print('juju --version ->', version) | ||
| if int(version[0]) >= 3: | ||
| (major, minor) = version.split('.')[0:2] |
There was a problem hiding this comment.
AAh such a silly mistake from me, you are right
| universal_newlines=True).strip() | ||
| print('juju --version ->', version) | ||
| if int(version[0]) >= 3: | ||
| (major, minor) = version.split('.')[0:2] |
There was a problem hiding this comment.
missing '' for . in the new change
3 participants
python-libjuju is released with a tight dependency with the underlying juju version, so when using juju-3.4 , python-libjuju-3.4 is needed, any mismatch will result in failures later.
This patch will still honor the semantic of TEST_JUJU3.