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

Added adapter filter #377

Merged
merged 21 commits into from
Apr 3, 2024
Merged

Added adapter filter #377

merged 21 commits into from
Apr 3, 2024

Conversation

anmenaga
Copy link
Collaborator

@anmenaga anmenaga commented Mar 29, 2024

PR Summary

Fix #274
Fix #368

  1. Removed duplicates from path list, so that same resources would not be processed multiple times;
  2. re-enabled WMI provider
  3. Added filter for adapters, so that in default scenarios 'List' operation on adapters is not executed, therefore reducing time.
Usage: dsc.exe resource list [OPTIONS] [RESOURCE_NAME]

Arguments:
  [RESOURCE_NAME]  Optional filter to apply to the list of resources

Options:
  -a, --adapter <ADAPTER_NAME>     Adapter filter to limit the resource search
  -d, --description <DESCRIPTION>  Description keyword to search for in the resource description
  -t, --tags <TAGS>                Tag to search for in the resource tags
  -f, --format <FORMAT>            The output format to use [possible values: json, pretty-json, yaml]
  -h, --help                       Print help

Examples:

  1. dsc resource list same as dsc resource list * - show all native resources, adapters, but not adapter-based resources (fastest);
  2. dsc resource list *Microsoft* - same as previous but also apply *Microsoft* filter;
  3. dsc resource list * -a * - show all adapter-based resources;
  4. dsc resource list * -a *PowerShell* - show all resources of adapters that match *PowerShell* filter
  5. dsc resource list *MSFT_PackageManagement* *PowerShell* - same as previous but also apply *MSFT_PackageManagement* filter to resource types;

@anmenaga anmenaga marked this pull request as draft March 29, 2024 19:34
Copy link
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

Seems like we need to makes sure we have test cases covering all your bullet points

dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Outdated Show resolved Hide resolved
dsc_lib/src/discovery/command_discovery.rs Show resolved Hide resolved
@anmenaga
Copy link
Collaborator Author

anmenaga commented Apr 1, 2024

Added tests.

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Apr 2, 2024

After trying out your branch, we should discuss some user experience options:

  • Currently, to use an adapter, you have to specify * or some resource name. I would prefer something like dsc resource list --adapter Microsoft.DSC/PowerShell and have it show all the PowerShell resources
  • When an adapter is specified, should the list ONLY contain resources for that adapter. I would think this would be expected unless you use a wildcard for the adapter, of course.

For docs, we should make the examples use wildcards in single quotes:

dsc resource list '*os*'

As on non-Windows, the shell will try to glob the wildcard and pass found files to the command causing it to fail.

cc @mgreenegit, @michaeltlombardi

@anmenaga
Copy link
Collaborator Author

anmenaga commented Apr 2, 2024

My thoughts:
1.

  • Currently, to use an adapter, you have to specify * or some resource name. I would prefer something like dsc resource list --adapter Microsoft.DSC/PowerShell and have it show all the PowerShell resources

It is shorter to type * for type filter than --adapter of the named parameter. I'd leave it as is.

  • When an adapter is specified, should the list ONLY contain resources for that adapter. I would think this would be expected unless you use a wildcard for the adapter, of course.

I agree with this; it is more intuitive than current behavior.

@mgreenegit
Copy link
Member

Get-dscresource allows the user to specify a module or individual resource. The operation is faster when it is scoped. That might be worth considering as well.

@SteveL-MSFT
Copy link
Member

My thoughts: 1.

  • Currently, to use an adapter, you have to specify * or some resource name. I would prefer something like dsc resource list --adapter Microsoft.DSC/PowerShell and have it show all the PowerShell resources

It is shorter to type * for type filter than --adapter of the named parameter. I'd leave it as is.

We would have a -a short name for --adapter. Particularly on Unix where you have to single-quote the wildcard so that it doesn't get globbed, my preference is to have adapter be a named arg and not require *.

@anmenaga
Copy link
Collaborator Author

anmenaga commented Apr 2, 2024

  • Currently, to use an adapter, you have to specify * or some resource name. I would prefer something like dsc resource list --adapter Microsoft.DSC/PowerShell and have it show all the PowerShell resources
  • When an adapter is specified, should the list ONLY contain resources for that adapter. I would think this would be expected unless you use a wildcard for the adapter, of course.

Updated code and PR description for both points.

dsc/tests/dsc_args.tests.ps1 Show resolved Hide resolved
dsc/tests/dsc_args.tests.ps1 Outdated Show resolved Hide resolved
dsc_lib/src/discovery/mod.rs Outdated Show resolved Hide resolved
tools/test_group_resource/tests/provider.tests.ps1 Outdated Show resolved Hide resolved
tools/test_group_resource/tests/provider.tests.ps1 Outdated Show resolved Hide resolved
anmenaga and others added 4 commits April 2, 2024 21:42
Co-authored-by: Steve Lee <slee@microsoft.com>
Co-authored-by: Steve Lee <slee@microsoft.com>
Co-authored-by: Steve Lee <slee@microsoft.com>
Co-authored-by: Steve Lee <slee@microsoft.com>
@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Apr 3, 2024
Merged via the queue into PowerShell:main with commit ffd58ca Apr 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants