First use apt to check if proxmoxer is installed.

[ZephOne]

If proxmoxer is installed via apt don't use pip to check this
dependency.
Otherwise pip also becomes a dependency which is not needed for
community.general.proxmox_kvm.
#15

[santiagomr]

Hi @ZephOne, thank you for submitting your comments and improvements.

I agree that it is an improvement to avoid the pip installation if proxmoxer can be installed via apt. Still, I don't understand how the code you send us in PR implements that improvement.

The conditionals you added will return true as long as the python-proxmoxer or python3-proxmoxer package is installed on the Proxmox node. So pip would still be installed anyway.

Don't you think it would be better to resolve tasks/dependencies.yml just with a single task using the apt module as follows?

- name: Verify if proxmoxer is installed
  ansible.builtin.apt:
    name: python3-proxmoxer
    state: present
  delegate_to: "{{ pve_api_host }}"

[ZephOne]

Hi,
the idea was to let the choice to people if they want to manage they python system dependencies using pip without forcing the use of apt to installed proxmoxer.

The conditionals you added will return true as long as the python-proxmoxer or python3-proxmoxer package is installed on the Proxmox node. So pip would still be installed anyway.

Well, I disagree with this point. If I check python-proxmoxer (or python3-proxmoxer) dependencies on Debian packages website it only depends on the package python3 so pip would not be installed.

Yet if the way proxmoxer is installed does not matter, and apt is the best way to install it, then what you propose is a better and simpler way to do it.

[ZephOne]

Hello,
I made some changes. @santiagomr do you think it is ok to be merged ?

[santiagomr]

Hello @ZephOne,

sorry for the delay in seeing the changes, you did well to mention me.

I was finishing the review and I verified that the change that we commented on actually works. Well... on Proxmox VE v7.

Unfortunately when testing it in previous versions of Proxmox VE (v5 and v6) that are supported by this role and by community.general.proxmox I noticed that there is no installation candidate for proxmoxer from the apt repositories, neither for python2 nor for python3 .

Although as we mentioned previously it is an improvement not needing to install pip, it does not seem to me that the improvement is very significant as to make this role stop supporting previous versions of Proxmox that many still have (we still have) in production.

There are still many features to add to the role that can be used in different versions of Proxmox. We will save this improvement for when support for previous versions of Proxmox VE is removed.

Thanks for using the role and sharing changes and suggestions.