python_requirements_info does not parse project[extra] syntax

Akasurde commented 4 years ago

From @aparedero on Nov 05, 2019 10:23

SUMMARY

python_requirements_info (previously python_requirements_facts) are not parsing pip packages if stated with project[extra] (similar to command pip install "project[extra]") + info

ISSUE TYPE

Bug Report

COMPONENT NAME

python_requirements_info

ANSIBLE VERSION

ansible 2.9.0
  config file = /home/user/git/my-awesome-project/ansible.cfg
  configured module search path = [u'/home/user/git/my-awesome-project/library']
  ansible python module location = /usr/local/lib/python2.7/dist-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.15+ (default, Oct  7 2019, 17:39:04) [GCC 7.4.0]

CONFIGURATION

N/A

OS / ENVIRONMENT

Linux Mint 19.1 Linux pc04 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

STEPS TO REPRODUCE

Add this task somewhere in your playbook. This task reads requirements.txt file from root folder and iterates over each line ensuring is installed. My first line in requirements.txt is ansible[azure], and that's where fails due to the module is not able to parse the extra packages.

About my loop, if there is a missing library, a mismatched version, or getting a "Failed to parse" message as follows I return this as error, avoiding the playbook continues.

- name: Ensure Python libraries are installed
  python_requirements_info:
    dependencies: "{{ item }}"
  with_lines: cat {{ base_dir }}/requirements.txt
  register: _py_facts
  failed_when: | 
      ( _py_facts.not_found is defined ) and ( _py_facts.not_found | length > 0 ) or
      ( _py_facts.mismatched is defined ) and ( _py_facts.mismatched | length > 0) or
      ( _py_facts.msg is defined) and ( _py_facts.msg | regex_search("Failed to parse"))

The requirements.txt in root folder

ansible[azure]>=2.8.0
ansible>=2.8.0
azure-datalake-store
[...]
pre-commit
openshift

EXPECTED RESULTS

TASK [Ensure Python libraries are installed] *************************************************************************************************************************************************************************************************
ok: [localhost -> localhost] => (item=ansible[azure]>=2.8.0)
ok: [localhost -> localhost] => (item=ansible>=2.8.0)
ok: [localhost -> localhost] => (item=azure-datalake-store)
[...]
ok: [localhost -> localhost] => (item=pre-commit)
ok: [localhost -> localhost] => (item=openshift)

ACTUAL RESULTS

TASK [Ensure Python libraries are installed] *************************************************************************************************************************************************************************************************
failed: [localhost -> localhost] (item=ansible[azure]>=2.8.0) => changed=false 
  ansible_loop_var: item
  failed_when_result: true
  item: ansible[azure]>=2.8.0
  msg: Failed to parse version requirement 'ansible[azure]>=2.8.0'. Must be formatted like 'ansible>2.6'
ok: [localhost -> localhost] => (item=ansible>=2.8.0)
ok: [localhost -> localhost] => (item=azure-datalake-store)
[...]
ok: [localhost -> localhost] => (item=pre-commit)
ok: [localhost -> localhost] => (item=openshift)

Copied from original issue: ansible/ansible#64435

Akasurde commented 4 years ago

@imjoseangel @bmillemathias Are you interested in working on this? Let me know.

ansibullbot commented 4 years ago

cc @MorrisA @bcoca @d-little @flynn1973 @gforster @kairoaraujo @marvin-sinister @mator @molekuul @ramooncamacho @willthames @wtcross click here for bot help

Akasurde commented 4 years ago

https://github.com/ansible-collections/community.general/blob/main/plugins/modules/system/python_requirements_info.py#L124 should handle additional square brackets.

imjoseangel commented 4 years ago

@imjoseangel @bmillemathias Are you interested in working on this? Let me know.

@Akasurde do you mean to help new contributors, don't you?

Akasurde commented 4 years ago

yes,

Antares1980 commented 4 years ago

Hi,

I would like to it. Thanks @imjoseangel for letting me know.

imjoseangel commented 4 years ago

@Antares1980 @aparedero

Be aware of the following here:

When requesting information about the [extra] syntax, you will need to match a new group.

In the example, you will have:

ansible[azure]>=2.8.0

pkg = 'ansible'
extras = '[azure]'
op = '>='
version = '2.8.0

if extras is not None: you need to handle if the required packages are installed with pkg_resources.require(pkg + extras) and handle the exceptions.

Note that it is important to manage not only DistributionNotFound but also UnknownExtra exceptions.

@Antares1980 Let's work together to guide you with the different tips.

Thank you!!

Antares1980 commented 4 years ago

Thanks @imjoseangel your help today to understand all the context has been exceptional.

As you said. To control the new group to check there are 4 places to review:

Regular expression as @Akasurde suggested to manage square brackets.
pkg, op, version = match.groups() should be changed to pkg, extra, op, version = match.groups(). Regular expression will allow only one extra by line.
Try block must handle UnknownExtra exception via Distribution.require(extras=()) method
if-else block to display correctly

Thanks again!

russoz commented 2 years ago

hi @Akasurde @aparedero I have been taking a look at this (including the abandoned PR). The major problem I see is that it does not need to be a package name (e.g. dnspython[DOH] - it actually requires requests). There is no trivial way, as far as I can see, to determine whether the extras have been installed or not.

ansible-collections / community.general