Skip to content
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

maven_artifact.py - add support for version ranges by using spec #54309

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
3 participants
@pjrm
Copy link
Contributor

pjrm commented Mar 24, 2019

SUMMARY

This PR allows to specify which versions of maven packages we want to download by using constraints.
The simple use case is when we want to download all patch versions released, but making sure that we don't download a major one.

Also I saw that other person have asked for this feature on #20003

The syntax supported for this module was based on this page.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

maven_artifact.py

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 24, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 25, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

test/sanity/validate-modules/ignore.txt:682:1: A102 Remove since "lib/ansible/modules/packaging/language/maven_artifact.py" passes "E324" test

click here for bot help

@ansibot ansibot added the ci_verified label Mar 25, 2019

@pjrm pjrm force-pushed the pjrm:maven_artifact_dynamic_version branch to 179c03d Mar 25, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Mar 25, 2019

@turb

This comment has been minimized.

Copy link
Contributor

turb commented Mar 25, 2019

Excellent, this is a needed one. Thank you!

It will need some extended testing, with several version patterns and several repositories. I am not familiar with unit testing with Ansible, but I think some unit tests on the regexps would be great too. I don't think the sanity test should be disabled.

To what extent do you think the Maven Specification is implemented?

@ansibot ansibot removed the needs_triage label Mar 25, 2019

@pjrm

This comment has been minimized.

Copy link
Contributor Author

pjrm commented Mar 25, 2019

Hi @turb,

Thanks for the reply.

Yes, I agree with you. I will try to find out how unit testings are done in Ansible, I have done some, but for sure it will be missing some cases.
Regarding the disabling of sanity test, I have done that because it failed from CI verification (you can see the ansibot comment, asking to remove it).

About the extent of Maven Specification, I'm not sure about it, but probably the first six ranges are implemented.

@ansibot ansibot added the stale_ci label Apr 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.