Skip to content

Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class which results in TypeError #10178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

umiruka
Copy link
Contributor

@umiruka umiruka commented May 26, 2025

SUMMARY

Fixes #8506 and complements PR #10145

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cobbler_system plugin

ADDITIONAL INFORMATION

When running a task with state param set to present (default value) as in the following example:

- name: Ensure cobbler systems were created
  community.general.cobbler_system:
    host: <hostname>
    username: cobbler
    password: <pwd>
    name: <profile_name>
    use_ssl: false
    state: present

I get the following error:

xmlrpc.client.Fault: <Fault 1: "<class 'TypeError'>:CobblerXMLRPCInterface.get_system_handle() takes 2 positional arguments but 3 were given">

This still happens after the fix #10145 because LooseVersion class does not compare the version numbers that are in different formats as expected. Cobbler's xmlrpc client library returns the version number for version 3.4.0 as 3.4 so as a result the comparison in line gives the following:

>>> LooseVersion('3.4') >= LooseVersion('3.4.0')
False

The expected result of the above comparison is True.

When using StrictVersion class we get the expected result:

>>> StrictVersion('3.4') >= StrictVersion('3.4.0')
True
Cobbler version
Cobbler 3.4.0
Ansible version
ansible [core 2.14.6]
community.general
community.general 10.6.0 

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added backport bug This issue/PR relates to a bug module module plugins plugin (any type) labels May 26, 2025
@felixfontein felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label May 27, 2025
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

@felixfontein felixfontein added backport-10 Automatically create a backport for the stable-10 branch and removed backport labels May 27, 2025
@umiruka umiruka changed the title Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class results in TypeError Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class which results in TypeError May 28, 2025
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 30, 2025
@felixfontein felixfontein merged commit 9717bac into ansible-collections:stable-10 Jun 2, 2025
138 checks passed
Copy link

patchback bot commented Jun 2, 2025

Backport to stable-10: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 9717bac on top of patchback/backports/stable-10/9717bac816fc4d0774405a1ebd32379fddf8e0dc/pr-10178

Backporting merged PR #10178 into stable-10

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/community.general.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-10/9717bac816fc4d0774405a1ebd32379fddf8e0dc/pr-10178 upstream/stable-10
  4. Now, cherry-pick PR Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class which results in TypeError #10178 contents into that branch:
    $ git cherry-pick -x 9717bac816fc4d0774405a1ebd32379fddf8e0dc
    If it'll yell at you with something like fatal: Commit 9717bac816fc4d0774405a1ebd32379fddf8e0dc is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x 9717bac816fc4d0774405a1ebd32379fddf8e0dc
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class which results in TypeError #10178 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-10/9717bac816fc4d0774405a1ebd32379fddf8e0dc/pr-10178
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jun 2, 2025
@felixfontein
Copy link
Collaborator

@umiruka thanks for fixing this!

felixfontein pushed a commit to felixfontein/community.general that referenced this pull request Jun 2, 2025
…n in some cases using LooseVersion class which results in TypeError (ansible-collections#10178)

* Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class

* Refactor code

* Add changelog for PR number 10178

* Update changelog to be more precise

* Use LooseVersion instead of StrictVersion to check cobbler's version in cobbler system module

* Update PR 10178 changelog description to be more accurate
@felixfontein
Copy link
Collaborator

I cherry-picked this into main as well. (It should have gone there right away and only then be backported to stable-10, but I apparently overlooked the wrong target branch...)

felixfontein pushed a commit that referenced this pull request Jun 2, 2025
…n in some cases using LooseVersion class which results in TypeError (#10178)

* Use StrictVersion class to avoid wrong version comparisons that happen in some cases using LooseVersion class

* Refactor code

* Add changelog for PR number 10178

* Update changelog to be more precise

* Use LooseVersion instead of StrictVersion to check cobbler's version in cobbler system module

* Update PR 10178 changelog description to be more accurate
@umiruka
Copy link
Contributor Author

umiruka commented Jun 3, 2025

I cherry-picked this into main as well. (It should have gone there right away and only then be backported to stable-10, but I apparently overlooked the wrong target branch...)

I will take note of that for next time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport backport-10 Automatically create a backport for the stable-10 branch bug This issue/PR relates to a bug module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants