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

Find: Adding additional file_type options #63117

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

Conversation

hspaans
Copy link

@hspaans hspaans commented Oct 3, 2019

SUMMARY

For bare metal server deployments we need to validate certain block devices exist before continuing. Adding multiple file types from stat() for the find module to use.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

find

ADDITIONAL INFORMATION

@ansibot
Copy link
Contributor

ansibot commented Oct 3, 2019

cc @bcoca
click here for bot help

@ansibot ansibot added affects_2.10 core_review feature files module needs_triage new_contributor support:core labels Oct 3, 2019
@ansibot
Copy link
Contributor

ansibot commented Oct 4, 2019

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

lib/ansible/modules/files/find.py:0:0: doc-choices-do-not-match-spec: Argument 'file_type' in argument_spec defines choices as (['any', 'directory', 'file', 'link']) but documentation defines choices as (['any', 'block', 'char', 'directory', 'fifo', 'file', 'link', 'socket'])

click here for bot help

@ansibot ansibot added ci_verified needs_revision core_review and removed core_review ci_verified needs_revision labels Oct 4, 2019
if pfilter(fsobj, params['patterns'], params['excludes'], params['use_regex']) and agefilter(st, now, age, params['age_stamp']):

r.update(statinfo(st))
filelist.append(r)
Copy link
Member

@bcoca bcoca Oct 4, 2019

Choose a reason for hiding this comment

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

since all new options are the same as 'directory' when it comes to the subsequent filter matching, just change it into one elif entry with file_type in ('directory', 'char' ...)

Copy link
Author

@hspaans hspaans Oct 4, 2019

Choose a reason for hiding this comment

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

That was my first setup until I saw link was also separate. Do you think we can drop the stat.IS* check and move this to be the last elif?

Copy link
Member

@bcoca bcoca Oct 4, 2019

Choose a reason for hiding this comment

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

link used to be different, not sure when it got made the same

Copy link
Author

@hspaans hspaans Nov 26, 2019

Choose a reason for hiding this comment

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

@bcoca did you had a chance to review the modification. To be honest, their isn't really a very elegant way to fix it imho.

@ansibot ansibot removed the needs_triage label Oct 4, 2019
@ansibot ansibot added needs_revision and removed core_review labels Oct 5, 2019
@ansibot ansibot added core_review and removed needs_revision labels Oct 5, 2019
@ansibot ansibot added the stale_ci label Oct 13, 2019
@hspaans hspaans closed this Oct 22, 2019
@hspaans hspaans reopened this Oct 22, 2019
@ansibot ansibot removed the stale_ci label Oct 22, 2019
@ansibot ansibot added the stale_ci label Oct 30, 2019
@hspaans hspaans requested a review from bcoca Mar 17, 2020
@hspaans hspaans closed this Mar 17, 2020
@hspaans hspaans reopened this Mar 17, 2020
@ansibot ansibot removed the stale_ci label Mar 17, 2020
@ansibot ansibot added the stale_ci label Mar 27, 2020
@ansibot ansibot added needs_rebase needs_revision and removed core_review labels May 16, 2020
@ansibot ansibot added pre_azp and removed stale_ci labels Dec 5, 2020
(stat.S_ISDIR(st.st_mode) and params['file_type'] == 'directory') or
(stat.S_ISFIFO(st.st_mode) and params['file_type'] == 'fifo') or
(stat.S_ISLNK(st.st_mode) and params['file_type'] == 'link') or
(stat.S_ISSOCK(st.st_mode) and params['file_type'] == 'socket')):
Copy link
Member

@bcoca bcoca Jan 12, 2022

Choose a reason for hiding this comment

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

I think an internal map with the file_type to 'stat operation' matching would be a cleaner way to implement this and then have:

statop = getattr(stat, FILE_MAP[params['file_type']] to find the function for comparison

@bcoca bcoca changed the title Adding additional file_type options Find: Adding additional file_type options Jul 20, 2022
@bcoca
Copy link
Member

bcoca commented Jul 20, 2022

waiting_on_contributor

@ansibot ansibot added the waiting_on_contributor label Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 feature files module needs_rebase needs_revision new_contributor pre_azp support:core waiting_on_contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants