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

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

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

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
* Replace os.path by Pathlib library & refactor
* Add new feature : Pattern matching on directory & file
* Add new integration test for cover new feature
@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
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
- Matches all files in a single directory, non-recursively, that match a pattern.
It calls Python's "glob" library.
- Matches all files in directory that match a pattern.
It calls Python's "`Path.glob <https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob>`_" library.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
It calls Python's "`Path.glob <https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob>`_" library.
It calls Python's "L(Path.glob, https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob)" library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

5 participants