Skip to content

Commit

Permalink
Fix ordering of setting progress data and writing it
Browse files Browse the repository at this point in the history
  • Loading branch information
@SteveL-MSFT
SteveL-MSFT committed Feb 19, 2025
1 parent 93e52da commit a087de5
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 56 deletions.
25 changes: 17 additions & 8 deletions dsc/tests/dsc_config_get.tests.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,21 @@ Describe 'dsc config get tests' {
$config_yaml = @"
`$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2024/04/config/document.json
resources:
- name: Echo
- name: Echo 1
type: Microsoft.DSC.Debug/Echo
properties:
output: hello
- name: Echo 2
type: Microsoft.DSC.Debug/Echo
properties:
output: world
"@
$config_yaml | dsc --progress-format json config get -f - 2> $TestDrive/ErrorStream.txt
$LASTEXITCODE | Should -Be 0
$lines = Get-Content $TestDrive/ErrorStream.txt
$ProgressMessagesFound = $false
$ProgressResultFound = $false
$InstanceOneFound = $false
$InstanceTwoFound = $false
foreach ($line in $lines) {
$jp = $line | ConvertFrom-Json
if ($jp.activity) { # if line is a progress message
Expand All @@ -78,15 +83,19 @@ Describe 'dsc config get tests' {
$ProgressMessagesFound = $true
}

if ($jp.percentComplete -eq 100 -and $jp.resourceType -eq 'Microsoft.DSC.Debug/Echo') {
$ProgressResultFound = $true
$jp.resourceName | Should -BeExactly 'Echo'
$jp.result | Should -Not -BeNullOrEmpty
$jp.result.output | Should -BeExactly 'hello'
if ($null -ne $jp.result -and $jp.resourceType -eq 'Microsoft.DSC.Debug/Echo') {
if ($jp.resourceName -eq 'Echo 1') {
$InstanceOneFound = $true
$jp.result.actualState.output | Should -BeExactly 'hello'
} elseif ($jp.resourceName -eq 'Echo 2') {
$InstanceTwoFound = $true
$jp.result.actualState.output | Should -BeExactly 'world'
}
}
}
$ProgressMessagesFound | Should -BeTrue
$ProgressResultFound | Should -BeTrue
$InstanceOneFound | Should -BeTrue
$InstanceTwoFound | Should -BeTrue
}

It 'contentVersion is ignored' {
Expand Down
34 changes: 15 additions & 19 deletions dsc_lib/src/configure/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ impl Configurator {
let resources = get_resource_invocation_order(&self.config, &mut self.statement_parser, &self.context)?;
let mut progress = ProgressBar::new(resources.len() as u64, self.progress_format)?;
for resource in resources {
progress.set_activity(format!("Get '{}'", resource.name).as_str());
progress.set_resource(&resource.name, &resource.resource_type, None);
progress.write_activity(format!("Get '{}'", resource.name).as_str());
let properties = self.invoke_property_expressions(resource.properties.as_ref())?;
let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else {
return Err(DscError::ResourceNotFound(resource.resource_type));
Expand All @@ -247,15 +247,13 @@ impl Configurator {
match &get_result {
GetResult::Resource(resource_result) => {
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&resource_result.actual_state)?);
progress.set_resource(&resource.name, &resource.resource_type, Some(&resource_result.actual_state));
},
GetResult::Group(group) => {
let mut results = Vec::<Value>::new();
for result in group {
results.push(serde_json::to_value(&result.result)?);
}
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), Value::Array(results.clone()));
progress.set_resource(&resource.name, &resource.resource_type, Some(&Value::Array(results.clone())));
},
}
let resource_result = config_result::ResourceGetResult {
Expand All @@ -271,10 +269,11 @@ impl Configurator {
),
name: resource.name.clone(),
resource_type: resource.resource_type.clone(),
result: get_result,
result: get_result.clone(),
};
result.results.push(resource_result);
progress.increment(1);
progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(get_result)?));
progress.write_increment(1);
}

result.metadata = Some(
Expand All @@ -301,8 +300,8 @@ impl Configurator {
let resources = get_resource_invocation_order(&self.config, &mut self.statement_parser, &self.context)?;
let mut progress = ProgressBar::new(resources.len() as u64, self.progress_format)?;
for resource in resources {
progress.set_activity(format!("Set '{}'", resource.name).as_str());
progress.set_resource(&resource.name, &resource.resource_type, None);
progress.write_activity(format!("Set '{}'", resource.name).as_str());
let properties = self.invoke_property_expressions(resource.properties.as_ref())?;
let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else {
return Err(DscError::ResourceNotFound(resource.resource_type));
Expand Down Expand Up @@ -370,18 +369,15 @@ impl Configurator {
match &set_result {
SetResult::Resource(resource_result) => {
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&resource_result.after_state)?);
progress.set_resource(&resource.name, &resource.resource_type, Some(&resource_result.after_state));
},
SetResult::Group(group) => {
let mut results = Vec::<Value>::new();
for result in group {
results.push(serde_json::to_value(&result.result)?);
}
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), Value::Array(results.clone()));
progress.set_resource(&resource.name, &resource.resource_type, Some(&Value::Array(results.clone())));
},
}

let resource_result = config_result::ResourceSetResult {
metadata: Some(
Metadata {
Expand All @@ -395,10 +391,11 @@ impl Configurator {
),
name: resource.name.clone(),
resource_type: resource.resource_type.clone(),
result: set_result,
result: set_result.clone(),
};
result.results.push(resource_result);
progress.increment(1);
progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(set_result)?));
progress.write_increment(1);
}

result.metadata = Some(
Expand All @@ -421,8 +418,8 @@ impl Configurator {
let resources = get_resource_invocation_order(&self.config, &mut self.statement_parser, &self.context)?;
let mut progress = ProgressBar::new(resources.len() as u64, self.progress_format)?;
for resource in resources {
progress.set_activity(format!("Test '{}'", resource.name).as_str());
progress.set_resource(&resource.name, &resource.resource_type, None);
progress.write_activity(format!("Test '{}'", resource.name).as_str());
let properties = self.invoke_property_expressions(resource.properties.as_ref())?;
let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else {
return Err(DscError::ResourceNotFound(resource.resource_type));
Expand All @@ -436,15 +433,13 @@ impl Configurator {
match &test_result {
TestResult::Resource(resource_test_result) => {
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&resource_test_result.actual_state)?);
progress.set_resource(&resource.name, &resource.resource_type, Some(&resource_test_result.actual_state));
},
TestResult::Group(group) => {
let mut results = Vec::<Value>::new();
for result in group {
results.push(serde_json::to_value(&result.result)?);
}
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), Value::Array(results.clone()));
progress.set_resource(&resource.name, &resource.resource_type, Some(&Value::Array(results.clone())));
},
}
let resource_result = config_result::ResourceTestResult {
Expand All @@ -460,10 +455,11 @@ impl Configurator {
),
name: resource.name.clone(),
resource_type: resource.resource_type.clone(),
result: test_result,
result: test_result.clone(),
};
result.results.push(resource_result);
progress.increment(1);
progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(test_result)?));
progress.write_increment(1);
}

result.metadata = Some(
Expand All @@ -488,8 +484,8 @@ impl Configurator {
let mut progress = ProgressBar::new(self.config.resources.len() as u64, self.progress_format)?;
let resources = self.config.resources.clone();
for resource in &resources {
progress.set_activity(format!("Export '{}'", resource.name).as_str());
progress.set_resource(&resource.name, &resource.resource_type, None);
progress.write_activity(format!("Export '{}'", resource.name).as_str());
let properties = self.invoke_property_expressions(resource.properties.as_ref())?;
let Some(dsc_resource) = self.discovery.find_resource(&resource.resource_type) else {
return Err(DscError::ResourceNotFound(resource.resource_type.clone()));
Expand All @@ -498,8 +494,8 @@ impl Configurator {
trace!("{}", t!("configure.mod.exportInput", input = input));
let export_result = add_resource_export_results_to_configuration(dsc_resource, Some(dsc_resource), &mut conf, input.as_str())?;
self.context.references.insert(format!("{}:{}", resource.resource_type, resource.name), serde_json::to_value(&export_result.actual_state)?);
progress.set_resource(&resource.name, &resource.resource_type, Some(&Value::Array(export_result.actual_state)));
progress.increment(1);
progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(export_result)?));
progress.write_increment(1);
}

conf.metadata = Some(self.get_result_metadata(Operation::Export));
Expand Down
12 changes: 6 additions & 6 deletions dsc_lib/src/discovery/command_discovery.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl ResourceDiscovery for CommandDiscovery {
};

let mut progress = ProgressBar::new(1, progress_format)?;
progress.set_activity(t!("discovery.commandDiscovery.progressSearching").to_string().as_str());
progress.write_activity(t!("discovery.commandDiscovery.progressSearching").to_string().as_str());

let mut resources = BTreeMap::<String, Vec<DscResource>>::new();
let mut adapters = BTreeMap::<String, Vec<DscResource>>::new();
Expand Down Expand Up @@ -235,7 +235,7 @@ impl ResourceDiscovery for CommandDiscovery {
}
}
}
progress.increment(1);
progress.write_increment(1);
debug!("Found {} matching non-adapter-based resources", resources.len());
self.resources = resources;
self.adapters = adapters;
Expand Down Expand Up @@ -268,14 +268,14 @@ impl ResourceDiscovery for CommandDiscovery {
};

let mut progress = ProgressBar::new(self.adapters.len() as u64, progress_format)?;
progress.set_activity("Searching for adapted resources");
progress.write_activity("Searching for adapted resources");

let mut adapted_resources = BTreeMap::<String, Vec<DscResource>>::new();

let mut found_adapter: bool = false;
for (adapter_name, adapters) in &self.adapters {
for adapter in adapters {
progress.increment(1);
progress.write_increment(1);

if !regex.is_match(adapter_name) {
continue;
Expand All @@ -284,7 +284,7 @@ impl ResourceDiscovery for CommandDiscovery {
found_adapter = true;
info!("Enumerating resources for adapter '{}'", adapter_name);
let mut adapter_progress = ProgressBar::new(1, progress_format)?;
adapter_progress.set_activity(format!("Enumerating resources for adapter '{adapter_name}'").as_str());
adapter_progress.write_activity(format!("Enumerating resources for adapter '{adapter_name}'").as_str());
let manifest = if let Some(manifest) = &adapter.manifest {
if let Ok(manifest) = import_manifest(manifest.clone()) {
manifest
Expand Down Expand Up @@ -336,7 +336,7 @@ impl ResourceDiscovery for CommandDiscovery {
};
}

adapter_progress.increment(1);
adapter_progress.write_increment(1);
debug!("Adapter '{}' listed {} resources", adapter_name, adapter_resources_count);
}
}
Expand Down
26 changes: 3 additions & 23 deletions dsc_lib/src/progress.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl ProgressBar {
///
/// * `delta` - The amount to increment the progress bar by
///
pub fn increment(&mut self, delta: u64) {
pub fn write_increment(&mut self, delta: u64) {
if self.format == ProgressFormat::None {
return;
}
Expand All @@ -131,7 +131,7 @@ impl ProgressBar {
}
}

/// Set the resource being operated on and write the progress
/// Set the resource being operated on
///
/// # Arguments
///
Expand All @@ -151,7 +151,7 @@ impl ProgressBar {
///
/// * `status` - The status of the operation
///
pub fn set_activity(&mut self, activity: &str) {
pub fn write_activity(&mut self, activity: &str) {
match self.format {
ProgressFormat::Json => {
self.progress_value.activity = Some(activity.to_string());
Expand Down Expand Up @@ -183,26 +183,6 @@ impl ProgressBar {
}
}

/// Set the position as progress through the items and write the progress
///
/// # Arguments
///
/// * `pos` - The position as progress through the items
///
pub fn set_position(&mut self, pos: u64) {
match self.format {
ProgressFormat::Json => {
self.item_position = pos;
self.set_percent_complete();
self.write_json();
},
ProgressFormat::Default => {
self.console_bar.pb_set_position(pos);
},
ProgressFormat::None => {}
}
}

fn write_json(&self) {
if let Ok(json) = serde_json::to_string(&self.progress_value) {
eprintln!("{json}");
Expand Down

0 comments on commit a087de5

Please sign in to comment.