Removed percentComplete and added totalItems and completedItems, add failed property to progress

Author: SteveL-MSFT

dsc/tests/dsc_config_get.tests.ps1

@@ -79,7 +79,8 @@ Describe 'dsc config get tests' {
             $jp = $line | ConvertFrom-Json
             if ($jp.activity) { # if line is a progress message
                 $jp.id | Should -Not -BeNullOrEmpty
-                $jp.percentComplete | Should -BeIn (0..100)
+                $jp.totalItems | Should -Not -BeNullOrEmpty
+                $jp.completedItems | Should -Not -BeNullOrEmpty
                 $ProgressMessagesFound = $true
             }
 
@@ -98,6 +99,53 @@ Describe 'dsc config get tests' {
         $InstanceTwoFound | Should -BeTrue
     }
 
+    It 'json progress returns correctly for failed resource' {
+        $config_yaml = @'
+            $schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2024/04/config/document.json
+            resources:
+            - name: Echo 1
+              type: Microsoft.DSC.Debug/Echo
+              properties:
+                output: hello
+            - name: ErrorTest
+              type: Test/ExitCode
+              properties:
+                exitCode: 1
+'@
+        dsc --progress-format json --trace-format json config get -i $config_yaml 2> $TestDrive/ErrorStream.txt
+        $LASTEXITCODE | Should -Be 2
+        $lines = Get-Content $TestDrive/ErrorStream.txt
+        $ProgressMessagesFound = $false
+        $InstanceOneFound = $false
+        $InstanceTwoFound = $false
+        foreach ($line in $lines) {
+            $jp = $line | ConvertFrom-Json
+            if ($jp.activity) { # if line is a progress message
+                $jp.id | Should -Not -BeNullOrEmpty
+                $jp.totalItems | Should -Not -BeNullOrEmpty
+                $jp.completedItems | Should -Not -BeNullOrEmpty
+                $ProgressMessagesFound = $true
+            }
+
+            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'
+                    $jp.failed | Should -BeNullOrEmpty
+                }
+            }
+            elseif ($null -ne $jp.failed -and $jp.resourceType -eq 'Test/ExitCode') {
+                if ($jp.resourceName -eq 'ErrorTest') {
+                    $InstanceTwoFound = $true
+                    $jp.result | Should -BeNullOrEmpty
+                }
+            }
+        }
+        $ProgressMessagesFound | Should -BeTrue
+        $InstanceOneFound | Should -BeTrue
+        $InstanceTwoFound | Should -BeTrue
+    }
+
     It 'contentVersion is ignored' {
         $config_yaml = @"
             `$schema: https://raw.githubusercontent.com/PowerShell/DSC/main/schemas/2024/04/config/document.json

dsc_lib/src/configure/mod.rs

@@ -232,7 +232,7 @@ 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_resource(&resource.name, &resource.resource_type, None);
+            progress.set_resource(&resource.name, &resource.resource_type);
             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 {
@@ -242,7 +242,14 @@ impl Configurator {
             let filter = add_metadata(&dsc_resource.kind, properties)?;
             trace!("filter: {filter}");
             let start_datetime = chrono::Local::now();
-            let get_result = dsc_resource.get(&filter)?;
+            let get_result = match dsc_resource.get(&filter) {
+                Ok(result) => result,
+                Err(e) => {
+                    progress.set_failed();
+                    progress.write_increment(1);
+                    return Err(e);
+                },
+            };
             let end_datetime = chrono::Local::now();
             match &get_result {
                 GetResult::Resource(resource_result) => {
@@ -272,7 +279,7 @@ impl Configurator {
                 result: get_result.clone(),
             };
             result.results.push(resource_result);
-            progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(get_result)?));
+            progress.set_result(&serde_json::to_value(get_result)?);
             progress.write_increment(1);
         }
 
@@ -295,12 +302,13 @@ impl Configurator {
     /// # Errors
     ///
     /// This function will return an error if the underlying resource fails.
+    #[allow(clippy::too_many_lines)]
     pub fn invoke_set(&mut self, skip_test: bool) -> Result<ConfigurationSetResult, DscError> {
         let mut result = ConfigurationSetResult::new();
         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_resource(&resource.name, &resource.resource_type, None);
+            progress.set_resource(&resource.name, &resource.resource_type);
             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 {
@@ -331,18 +339,43 @@ impl Configurator {
             if exist || dsc_resource.capabilities.contains(&Capability::SetHandlesExist) {
                 debug!("{}", t!("configure.mod.handlesExist"));
                 start_datetime = chrono::Local::now();
-                set_result = dsc_resource.set(&desired, skip_test, &self.context.execution_type)?;
+                set_result = match dsc_resource.set(&desired, skip_test, &self.context.execution_type) {
+                    Ok(result) => result,
+                    Err(e) => {
+                        progress.set_failed();
+                        progress.write_increment(1);
+                        return Err(e);
+                    },
+                };
                 end_datetime = chrono::Local::now();
             } else if dsc_resource.capabilities.contains(&Capability::Delete) {
                 if self.context.execution_type == ExecutionKind::WhatIf {
                     // TODO: add delete what-if support
                     return Err(DscError::NotSupported(t!("configure.mod.whatIfNotSupportedForDelete").to_string()));
                 }
                 debug!("{}", t!("configure.mod.implementsDelete"));
-                let before_result = dsc_resource.get(&desired)?;
+                let before_result = match dsc_resource.get(&desired) {
+                    Ok(result) => result,
+                    Err(e) => {
+                        progress.set_failed();
+                        progress.write_increment(1);
+                        return Err(e);
+                    },
+                };
                 start_datetime = chrono::Local::now();
-                dsc_resource.delete(&desired)?;
-                let after_result = dsc_resource.get(&desired)?;
+                if let Err(err) = dsc_resource.delete(&desired) {
+                    progress.set_failed();
+                    progress.write_increment(1);
+                    return Err(err);
+                }
+                let after_result = match dsc_resource.get(&desired) {
+                    Ok(result) => result,
+                    Err(e) => {
+                        progress.set_failed();
+                        progress.write_increment(1);
+                        return Err(e);
+                    },
+                };
                 // convert get result to set result
                 set_result = match before_result {
                     GetResult::Resource(before_response) => {
@@ -394,7 +427,7 @@ impl Configurator {
                 result: set_result.clone(),
             };
             result.results.push(resource_result);
-            progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(set_result)?));
+            progress.set_result(&serde_json::to_value(set_result)?);
             progress.write_increment(1);
         }
 
@@ -418,7 +451,7 @@ 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_resource(&resource.name, &resource.resource_type, None);
+            progress.set_resource(&resource.name, &resource.resource_type);
             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 {
@@ -428,7 +461,14 @@ impl Configurator {
             let expected = add_metadata(&dsc_resource.kind, properties)?;
             trace!("{}", t!("configure.mod.expectedState", state = expected));
             let start_datetime = chrono::Local::now();
-            let test_result = dsc_resource.test(&expected)?;
+            let test_result = match dsc_resource.test(&expected) {
+                Ok(result) => result,
+                Err(e) => {
+                    progress.set_failed();
+                    progress.write_increment(1);
+                    return Err(e);
+                },
+            };
             let end_datetime = chrono::Local::now();
             match &test_result {
                 TestResult::Resource(resource_test_result) => {
@@ -458,7 +498,7 @@ impl Configurator {
                 result: test_result.clone(),
             };
             result.results.push(resource_result);
-            progress.set_resource(&resource.name, &resource.resource_type, Some(&serde_json::to_value(test_result)?));
+            progress.set_result( &serde_json::to_value(test_result)?);
             progress.write_increment(1);
         }
 
@@ -484,17 +524,24 @@ 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_resource(&resource.name, &resource.resource_type, None);
+            progress.set_resource(&resource.name, &resource.resource_type);
             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()));
             };
             let input = add_metadata(&dsc_resource.kind, properties)?;
             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())?;
+            let export_result = match add_resource_export_results_to_configuration(dsc_resource, Some(dsc_resource), &mut conf, input.as_str()) {
+                Ok(result) => result,
+                Err(e) => {
+                    progress.set_failed();
+                    progress.write_increment(1);
+                    return Err(e);
+                },
+            };
             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(&serde_json::to_value(export_result)?));
+            progress.set_result(&serde_json::to_value(export_result)?);
             progress.write_increment(1);
         }
 

dsc_lib/src/progress.rs

@@ -24,36 +24,42 @@ pub enum ProgressFormat {
 }
 
 #[derive(Default, Debug, Clone, Serialize)]
+#[serde(rename_all = "camelCase")]
 pub struct Progress {
     /// The unique identifier for the operation.
     pub id: String,
     /// The activity being performed.
     pub activity: Option<String>,
-    /// The percentage of the operation that is complete from 0 to 100.
-    #[serde(rename = "percentComplete")]
-    pub percent_complete: u8,
+    /// The total number of items to process.
+    pub total_items: u64,
+    /// The number of items processed.
+    pub completed_items: u64,
     /// The status of the operation.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub status: Option<String>,
     /// The number of seconds remaining in the operation.
-    #[serde(rename = "secondsRemaining", skip_serializing_if = "Option::is_none")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub seconds_remaining: Option<u64>,
     /// The name of the resource being operated on.
-    #[serde(rename = "resourceName", skip_serializing_if = "Option::is_none")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub resource_name: Option<String>,
     /// The type of the resource being operated on.
-    #[serde(rename = "resourceType", skip_serializing_if = "Option::is_none")]
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub resource_type: Option<String>,
     /// The result of the operation.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub result: Option<Value>,
+    /// Indicates if the operation failed.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub failed: Option<bool>,
 }
 
 impl Progress {
     #[must_use]
-    pub fn new() -> Progress {
+    pub fn new(total_items: u64) -> Progress {
         Progress {
             id: Uuid::new_v4().to_string(),
+            total_items,
             ..Default::default()
         }
     }
@@ -62,8 +68,6 @@ impl Progress {
 pub struct ProgressBar {
     progress_value:  Progress,
     console_bar: Span,
-    item_count: u64,
-    item_position: u64,
     format: ProgressFormat
 }
 
@@ -84,21 +88,19 @@ impl ProgressBar {
     ///
     /// Fails if progress style for console rendering can't be set.
     ///
-    pub fn new(item_count: u64, format: ProgressFormat) -> Result<ProgressBar, DscError> {
+    pub fn new(total_items: u64, format: ProgressFormat) -> Result<ProgressBar, DscError> {
         let bar = warn_span!("");
         if format == ProgressFormat::Default {
             bar.pb_set_style(&ProgressStyle::with_template(
                 "{spinner:.green} [{elapsed_precise:.cyan}] [{bar:40.cyan/blue}] {pos:>7}/{len:7} {msg:.yellow}"
             )?);
-            bar.pb_set_length(item_count);
+            bar.pb_set_length(total_items);
             let _guard = bar.enter();
         }
 
         Ok(ProgressBar {
-            progress_value: Progress::new(),
+            progress_value: Progress::new(total_items),
             console_bar: bar,
-            item_count,
-            item_position: 0,
             format
         })
     }
@@ -114,7 +116,7 @@ impl ProgressBar {
             return;
         }
 
-        self.item_position += delta;
+        self.progress_value.completed_items += delta;
         if self.format == ProgressFormat::Json {
             self.write_json();
         } else {
@@ -130,10 +132,29 @@ impl ProgressBar {
     /// * `resource_type` - The type of the resource being operated on
     /// * `result` - The result of the operation
     ///
-    pub fn set_resource(&mut self, name: &str, resource_type: &str, result: Option<&Value>) {
+    pub fn set_resource(&mut self, name: &str, resource_type: &str) {
         self.progress_value.resource_name = Some(name.to_string());
         self.progress_value.resource_type = Some(resource_type.to_string());
-        self.progress_value.result = result.cloned();
+        self.progress_value.result = None;
+        self.progress_value.failed = None;
+    }
+
+    /// Set the result of the operation. This will clear any error.
+    ///
+    /// # Arguments
+    ///
+    /// * `result` - The result of the operation
+    ///
+    pub fn set_result(&mut self, result: &Value) {
+        self.progress_value.failed = None;
+        self.progress_value.result = Some(result.clone());
+    }
+
+    /// Indicate that the operation failed.  This will clear any result.
+    ///
+    pub fn set_failed(&mut self) {
+        self.progress_value.result = None;
+        self.progress_value.failed = Some(true);
     }
 
     /// Set the status of the operation and write the progress
@@ -161,10 +182,10 @@ impl ProgressBar {
     ///
     /// * `len` - The number of total items to complete
     ///
-    pub fn set_length(&mut self, len: u64) {
+    pub fn set_total_items(&mut self, len: u64) {
         match self.format {
             ProgressFormat::Json => {
-                self.item_count = len;
+                self.progress_value.total_items = len;
             },
             ProgressFormat::Default => {
                 self.console_bar.pb_set_length(len);
@@ -174,14 +195,6 @@ impl ProgressBar {
     }
 
     fn write_json(&mut self) {
-        if self.item_count  > 0 {
-            self.progress_value.percent_complete = if self.item_position >= self.item_count {
-                100
-            } else {
-                u8::try_from((self.item_position * 100) / self.item_count).unwrap_or(100)
-            };
-        }
-
         if let Ok(json) = serde_json::to_string(&self.progress_value) {
             eprintln!("{json}");
         } else {