chocolatey / chocolatey-ansible

The Chocolatey module collection for Ansible
GNU General Public License v3.0
47 stars 29 forks source link

(108) Add filter parameter to win_chocolatey_facts module #129

Closed Windos closed 1 year ago

Windos commented 1 year ago

Description Of Changes

This PR adds a gather_subset parameter to the win_chocolatey_facts module which allows the user to "gather" only a subset of facts.

Motivation and Context

Currently the win_chocolatey_facts module gets ALL facts available to it, including:

This means there is a lot of information being complied even if the user doesn't need it. This can be especially egregious when pulling the outdated packages when the target has a large number of packages installed.

gather_subset is an established pattern when gathering general Ansible facts, see Gathers facts about remote hosts, and the implementation in this module emulates this existing pattern as closely as makes sense.

Testing

I have built and tested the collection locally, it has also been tested via the Azure DevOps pipeline: Azure Pipeline Test Results (Passing)

Operating Systems Testing

N/A

Change Types Made

Change Checklist

Related Issue

Fixes #108

Windos commented 1 year ago

@vexx32, most of your comments above will be addressed by this so I'll pop it here instead of replying to each individually.

As a first pass I was laser focused on matching the existing gather_subset behaviour for when Ansible is gathering its facts and... yeah it's odd. I think it makes more sense when there are 60+ options (doubled to 120+ when you include the ! variants of each.

They also have, in addition to all, a minimum subset min, so in order to say you only want one subset you need to do:

gather_subset:
  - '!all'
  - '!min'
  - interfaces

Having thought on it over the weekend, we only have 5 fact subsets, and so don't need to be as... complicated.

I'm going to push a "take 2" as a second commit (so that we can compare this to that, rather than completely wiping away this one) for consideration which I think will match the needs of Chocolatey users a lot better.

vexx32 commented 1 year ago

@Windos yeah, that's fair. I think we're okay simplifying it a good bit, as you say there's only a handful of options after all. I think those changes look good to me, apart from the minor thing with the naming of the tests. đŸ’–

Windos commented 1 year ago

@vexx32 - I have fixed up that last comment, squashed the commits, and this is ready for final review/merge at your leisure.