-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add New Module file_remove #11032
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
base: main
Are you sure you want to change the base?
Add New Module file_remove #11032
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
5b8e67c to
fc19d13
Compare
fc19d13 to
c2250a8
Compare
|
The typing errors are unrelated and have been fixed in another PR. |
482a0e2 to
cdb95e8
Compare
kginonredhat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
russoz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @shahargolshani Thanks for your contribution!
Overall it looks good, just a few adjustments. I have not checked the logic of the tests, but the use case for this is not very complex, I am trusting they are covering what's needed.
| try: | ||
| re.compile(pattern) | ||
| except re.error as e: | ||
| module.fail_json(msg=f"Invalid regular expression pattern: {to_native(e)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is redundant, since we no longer support Python 2, more occurrences around the code should be adjusted.
| module.fail_json(msg=f"Invalid regular expression pattern: {to_native(e)}") | |
| module.fail_json(msg=f"Invalid regular expression pattern: {e}") |
| # SPDX-License-Identifier: GPL-3.0-or-later | ||
|
|
||
| azp/posix/3 | ||
| destructive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguably not, since everything is created and destroyed within a temporary directory. @felixfontein what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this isn't needed. The tests aren't installing/removing/modifying packages, system config files, services etc. or doing similar destructive things, which this aliases entry is for.
felixfontein
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
| - This module removes files from a specified directory that match a given pattern. | ||
| - The pattern can include wildcards and regular expressions. | ||
| - By default, only files in the specified directory are removed (non-recursive). | ||
| - Use the O(recursive) option to search and remove files in subdirectories. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think some fewer paragraphs would be better here:
| - This module removes files from a specified directory that match a given pattern. | |
| - The pattern can include wildcards and regular expressions. | |
| - By default, only files in the specified directory are removed (non-recursive). | |
| - Use the O(recursive) option to search and remove files in subdirectories. | |
| - This module removes files from a specified directory that match a given pattern. | |
| The pattern can include wildcards and regular expressions. | |
| - By default, only files in the specified directory are removed (non-recursive). | |
| Use the O(recursive) option to search and remove files in subdirectories. |
| check_mode: | ||
| description: Can run in check_mode and return changed status without modifying the target. | ||
| support: full | ||
| diff_mode: | ||
| description: Will return details on what has changed (or possibly needs changing in check_mode). | ||
| support: full |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the descriptions and use the appropriate doc fragment (see basically every other module in this collection).
| pattern: | ||
| description: | ||
| - Pattern to match files for removal. | ||
| - Supports wildcards (*, ?, [seq], [!seq]) for glob-style matching. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Supports wildcards (*, ?, [seq], [!seq]) for glob-style matching. | |
| - Supports wildcards (V(*), V(?), V([seq]), V([!seq])) for glob-style matching. |
| - V(file) - remove only regular files. | ||
| - V(link) - remove only symbolic links. | ||
| - V(any) - remove both files and symbolic links. | ||
| type: str | ||
| choices: ['file', 'link', 'any'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - V(file) - remove only regular files. | |
| - V(link) - remove only symbolic links. | |
| - V(any) - remove both files and symbolic links. | |
| type: str | |
| choices: ['file', 'link', 'any'] | |
| type: str | |
| choices: | |
| file: remove only regular files. | |
| link: remove only symbolic links. | |
| any: remove both files and symbolic links. |
|
|
||
| msg: | ||
| description: Status message. | ||
| type: str | ||
| returned: always | ||
| sample: "Removed 2 files matching pattern '*.log'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally don't document standard return values.
| msg: | |
| description: Status message. | |
| type: str | |
| returned: always | |
| sample: "Removed 2 files matching pattern '*.log'" |
SUMMARY
This PR introduces a new module,
file_remove, designed to idempotently delete a single file from a target host.Rationale: This module serves as an enhancement for many file deletion cases not currently supported by ansible.builtin.file with state=absent.
Pattern-based deletion: It allows using regular expressions to remove multiple files that match a specific pattern (e.g., all *.log or *.tmp files) in a single task.
Design: This avoids the common workaround of using ansible.builtin.find, registering the results, and then looping over them with ansible.builtin.file. This module performs that entire operation idempotently in one step.
This new module provides a more powerful and intuitive interface for these common "clean-up" style operations.
changelog: New Module
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION