Skip to content

Improve fileglob lookup - Pattern matching on directory & file #83600

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: devel
Choose a base branch
from
Open

Improve fileglob lookup - Pattern matching on directory & file #83600

wants to merge 4 commits into from
+168 −30

Conversation

HS-157
Copy link

@HS-157 HS-157 commented Jul 13, 2024

  • Replace os.path by Pathlib library & refactor
  • Add new feature : Pattern matching on directory & file
  • Add new integration test for cover new feature
SUMMARY

Hi, I modify and improve fileglob lookup for adding pattern matching to folder and keeping the previous behaviors.
I made this change because I think it's a shame that this lookup doesn't have these features and that it's laborious to get the same behavior by other means.

I replace os.path library by Pathlib because it's a better library for manipulate filesystem paths. This library was added in Python 3.4. I add new integration test and examples for cover this new features. I run sanity and integration test for fileglob, there are no errors.

I welcome any feedback to improve this PR. Thanks.

ISSUE TYPE
  • Feature Pull Request
  • Test Pull Request

@ansibot ansibot added feature This issue/PR relates to a feature request. test This PR relates to tests. needs_triage Needs a first human triage before being processed. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 13, 2024
@ansibot
Copy link
Contributor

ansibot commented Jul 13, 2024

The test ansible-test sanity --test pylint [explain] failed with 6 errors:

test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo1/bar.py:1:0: disallowed-name: Disallowed name "bar"
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo2/bar.py:1:0: disallowed-name: Disallowed name "bar"
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo23/bar.py:1:0: disallowed-name: Disallowed name "bar"
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo3/bar.py:1:0: disallowed-name: Disallowed name "bar"
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo4/bar.py:1:0: disallowed-name: Disallowed name "bar"
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/toto/toto.py:1:0: disallowed-name: Disallowed name "toto"

The test ansible-test sanity --test boilerplate [explain] failed with 11 errors:

test/integration/targets/lookup_fileglob/recursive/outside_role/hi.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/bor.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/hello.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo1/bar.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo2/bar.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo23/bar.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo3/bar.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/ooo4/bar.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/hello.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/toto/toto.py:0:0: missing: from __future__ import annotations
test/integration/targets/lookup_fileglob/recursive/roles/get_file/not_files_dir/coucou.py:0:0: missing: from __future__ import annotations

The test ansible-test sanity --test no-unwanted-characters [explain] failed with 24 errors:

lib/ansible/plugins/lookup/fileglob.py:38:58: use an ASCII space instead of a Unicode no-break space
lib/ansible/plugins/lookup/fileglob.py:41:61: use an ASCII space instead of a Unicode no-break space
lib/ansible/plugins/lookup/fileglob.py:47:62: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/readme.md:1:2: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foobar.md:1:2: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/toto/2-middle.md:1:2: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/toto/3-end.md:1:2: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/not_files_dir/coucou.md:1:2: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:4:38: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:9:39: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:14:36: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:19:37: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:24:52: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:29:44: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:34:37: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:39:39: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:44:34: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:46:41: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:49:44: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:51:44: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:54:45: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:59:51: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/recursive/roles/get_file/tasks/main.yml:64:39: use an ASCII space instead of a Unicode no-break space
test/integration/targets/lookup_fileglob/runme.sh:20:2: use an ASCII space instead of a Unicode no-break space

The test ansible-test sanity --test pymarkdown [explain] failed with 5 errors:

test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foo/readme.md:1:1: no-missing-space-atx: No space present after the hash character on a possible Atx Heading.
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/foobar.md:1:1: no-missing-space-atx: No space present after the hash character on a possible Atx Heading.
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/toto/2-middle.md:1:1: no-missing-space-atx: No space present after the hash character on a possible Atx Heading.
test/integration/targets/lookup_fileglob/recursive/roles/get_file/files/toto/3-end.md:1:1: no-missing-space-atx: No space present after the hash character on a possible Atx Heading.
test/integration/targets/lookup_fileglob/recursive/roles/get_file/not_files_dir/coucou.md:1:1: no-missing-space-atx: No space present after the hash character on a possible Atx Heading.

click here for bot help

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 13, 2024
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Jul 18, 2024
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 25, 2024
@HS-157 HS-157 changed the title Improve fileglob lookup Improve fileglob lookup - Pattern matching on directory & file Oct 31, 2024
HS-157 added 2 commits October 31, 2024 15:29
@HS-157
Improve fileglob lookup
550e1e5 
* Replace os.path by Pathlib library & refactor
* Add new feature : Pattern matching on directory & file
* Add new integration test for cover new feature
@HS-157
Fix test
be6e985
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 31, 2024
@HS-157
Fix path object convertion to string
2b7ed0f 
For Windows path, there is a difference between `str(p)` &
`p.as_posix()`
@mscherer
Copy link
Contributor

mscherer commented Nov 1, 2024

Seems correct, I did a review during pycon.fr with submitter and @pilou-

The behavior did seems curious, but that was the original one, so I would say LGTM.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 15, 2024
felixfontein
felixfontein reviewed Feb 5, 2025
View reviewed changes
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 6, 2025
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

Assignees
No one assigned
Labels
feature This issue/PR relates to a feature request. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@HS-157 @ansibot @mscherer @felixfontein @s-hertel