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

Implement 'pathglob' filter plugin and lookup plugin #17483

Closed
wants to merge 2 commits into from
Closed

Implement 'pathglob' filter plugin and lookup plugin #17483

wants to merge 2 commits into from

Conversation

charlienewey
Copy link

@charlienewey charlienewey commented Sep 9, 2016

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

  • pathglob filter plugin
  • pathglob lookup plugin

SUMMARY

In response to comments in #17136 and #17269, alternative pathglob filter and lookup plugins to match both files and directories, or directories only (if specified in glob, e.g. {{ "/some/path/*/" | pathglob}}).. This feature is suggested in #17480.

@charlienewey charlienewey changed the title Implement 'glob' filter plugin and lookup plugin Implement 'pathglob' filter plugin and lookup plugin Sep 9, 2016
@bcoca bcoca added feature_pull_request new_plugin This PR includes a new plugin. labels Sep 9, 2016
@charlienewey
Copy link
Author

@bcoca I noticed you closed the issues referenced in this PR today - is there any movement on accepting this?

@tknerr
Copy link

tknerr commented Oct 28, 2016

@bcoca @charlienewey would definitely be nice to have a dirglob and / or pathglob in addition to the fileglob filter and lookup plugins.

Example here:

- name: List all currently configured nodes in nodes directory
  command: find /var/lib/jenkins/nodes -mindepth 1 -maxdepth 1 -type d -exec basename {} \;
  register: existing_nodes
  changed_when: False

- name: Remove all unmanaged slave nodes from the plugins directory
  shell: rm -rf /var/lib/jenkins/nodes/{{ item }}
  with_items: "{{ existing_nodes.stdout_lines }}"
  when: item not in "{{ jenkins_slave_nodes | default([]) | map(attribute='name') | list }}"

Where I'd rather like to write simply:

- name: Remove all unmanaged slave nodes from the plugins directory
  shell: rm -rf /var/lib/jenkins/nodes/{{ item | basename }}
  with_dirglob:
    - /var/lib/jenkins/nodes/*
  when: "{{ item | basename }}" not in "{{ jenkins_slave_nodes | default([]) | map(attribute='name') | list }}"

@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 13, 2016
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 2, 2017
@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 Apr 11, 2017
@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jun 29, 2017
@calfonso
Copy link
Contributor

calfonso commented Aug 2, 2017

@charlienewey is there any reason you can't execute a shell command to find the dirs and files, register the output, and reference the registered in the next task?

@charlienewey
Copy link
Author

@calfonso That would be inconsistent behaviour, no? The fileglob module does the same thing for files, I'm just making the behaviour consistent. Besides, it's inconvenient to do the above as you mentioned.

@bcoca
Copy link
Member

bcoca commented Aug 9, 2017

I would prefer each plugin in it's own PR, mostly cause filter is much easier/saner to merge.

filter:

  • implementation is fine, we should have strong docs letting user know the paths resolve ON CONTROLLER.

lookup:

  • docs issue, same as filter .. but more
  • fileglob is a bit murky and this one has same issues, when it is a relative lookup going through dwim ... people have different expectations from what really happens. There are many diff ways people expect it to work but only one real way it does. This can either be VERY clearly stated in docs or use an option to alter between the diff expectations.

@bcoca bcoca added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 9, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@djmattyg007
Copy link
Contributor

@calfonso thatcs really awkward to do, and find is a very unintuitive command. On the other hand, most people generally understand globbing a lot more easily. I'd definitely appreciate this being in the core.

@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Dec 20, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Feb 6, 2018
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 2, 2018
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 18, 2018
@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Nov 26, 2018
@maxamillion
Copy link
Contributor

Hi!

Thanks very much for your submission to Ansible. It sincerely means a lot to us that you've taken time to contribute.

Unfortunately, we're not sure if we want this feature in the program, and I don't want this to seem confrontational. Our reasons for this are because we already have the filetree lookup plugin that satisfies this use case.

https://docs.ansible.com/ansible/latest/plugins/lookup/filetree.html

However, we're absolutely always up for discussion. Since this is a really busy project, we don't always see comments on closed tickets, but want to encourage
open dialog. You can stop by the development list, and we'd be glad to talk about it - and we might even be persuaded otherwise!

In the future, sometimes starting a discussion on the development list prior to implementing a feature can make getting things included a little easier, but it's not always necessary.

Thank you once again for this and your interest in Ansible!

@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 c:plugins/filter c:plugins/lookup feature This issue/PR relates to a feature request. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. new_plugin This PR includes a new plugin. plugins/filter plugins/lookup stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants