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

Enable group resource output schema to simplify config output that uses groups #318

Merged
merged 15 commits into from
Feb 27, 2024

Conversation

SteveL-MSFT
Copy link
Member

PR Summary

  • New schema for group resources for get, set, and test
  • dsc as a resource has new --as-group to use this new schema
  • dsc as a resource has new --as-get to use with AssertionGroup so that a test operation still returns a get output to conform with new schema
  • new debug and trace needed to identify issue
  • new tests for Group which also uses nested groups
  • fix input validation to use validate method for group resources instead of relying only on schema
  • fix config validation so that resources only get their portion of the config instead of the whole config doc (which resulted in unintended infinite recursion when dsc was the resource)
  • added test for brew example resource and yaml
  • update osinfo readme to match code
  • updated some existing debug to be traces conforming with new standard

PR Context

Addresses the get part of #266, set and test will be addressed in separate PRs

type: Test/Echo
properties:
output: Last
dependsOn:
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like too short ident for dependsOn.

Copy link
Member Author

Choose a reason for hiding this comment

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

This dependsOn is for the Last Group whereas the lines above are the resources within the group, so the indent here is correct

@@ -7,14 +7,17 @@
"executable": "dsc",
"args": [
"config",
"test"
"--as-group",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would help if there were short examples of format for both --as-group and --as-get in PR description (and docs).

Copy link
Member Author

Choose a reason for hiding this comment

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

I provided descriptions of those two switches in the PR description. They won't be doc'd since they are hidden and intended to be only used by dsc itself.

@@ -102,6 +147,27 @@ pub struct ResourceTestResult {
pub result: TestResult,
}

#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
#[serde(deny_unknown_fields)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a strong reason to deny_unknown_fields ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can evaluate if we want to allow arbitrary fields later if a scenario comes up

@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Feb 27, 2024
Merged via the queue into PowerShell:main with commit af3403e Feb 27, 2024
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the config-result branch February 27, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants