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

Add parameters support #291

Merged
merged 7 commits into from
Jan 6, 2024
Merged

Conversation

SteveL-MSFT
Copy link
Member

PR Summary

Added:

  • dsc CLI can accept parameters JSON/YAML as string or file
  • parameters are validated against constraints (some TODOs in the code to fill in later)
  • context added to configurator keeping necessary state for parameters but also future use for variables/references

The osinfo example YAMLs currently do not work due to schema validation happening before expression execution

PR Context

Fix #49

use serde_json::Value;
use std::collections::HashMap;

pub struct Context {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If params can potentially be used in direct resource invocations (in addition to configurations) then this file probably should be moved to a more shared place.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no intent to have params supported for direct resource invocation, it's a feature of config only at this time

@@ -4,6 +4,6 @@ version = "0.1.0"
edition = "2021"

[dependencies]
sysinfo = "*"
sysinfo = "0.30"
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Are this changes in "process" related to this PR/parameters?
  2. why the "sysinfo" package version change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, because the *, CI picked up a different older version than what I was building on my local system so some of the syntax changed between versions. Having explicit version keeps it consistent so it builds both locally and in CI.

/// # Errors
///
/// * `DscError::Validation` if the value does not match the constraints.
pub fn check_number(name: &str, value: &Value, constraint: &Parameter) -> Result<(), DscError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rename this to be more specific like something like "check_number_limits" or "check_max_min" .

Copy link
Member Author

Choose a reason for hiding this comment

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

agree on naming, will make this change.

@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Jan 6, 2024
Merged via the queue into PowerShell:main with commit 6581288 Jan 6, 2024
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the parameters branch January 6, 2024 19:42
michaeltlombardi added a commit to michaeltlombardi/DSC that referenced this pull request Feb 5, 2024
This change adds documentation to the CLI and schema reference
for the parameters support  implementation in PowerShell#291 and PowerShell#294. It
also updates the changelog with the relevant information.
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.

Implement parameters support
2 participants