Remove on_success from DefaultHandlers

didier-wenzek · didier-wenzek · commit dfb916dc55cb · 2024-08-21T09:45:42.000+02:00
The idea is to force the users to be explicit on which state is to be
used on a successful action. Using the terminal successful test being
likely an error on most cases.

Signed-off-by: Didier Wenzek &lt;didier.wenzek@free.fr&gt;
diff --git a/crates/core/tedge_api/src/workflow/error.rs b/crates/core/tedge_api/src/workflow/error.rs
@@ -37,6 +37,9 @@ pub enum ScriptDefinitionError {
 
     #[error("Invalid exit code range '{from}-{to}' as {from}>{to}")]
     IncorrectRange { from: u8, to: u8 },
+
+    #[error("No handler is provided for 'on_success'")]
+    MissingOnSuccessHandler,
 }
 
 /// Error related to state excerpt definitions
diff --git a/crates/core/tedge_api/src/workflow/script.rs b/crates/core/tedge_api/src/workflow/script.rs
@@ -346,21 +346,18 @@ impl IterateHandlers {
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct DefaultHandlers {
     pub timeout: Option<Duration>,
-    pub on_success: GenericStateUpdate,
     pub on_error: GenericStateUpdate,
     pub on_timeout: GenericStateUpdate,
 }
 
 impl DefaultHandlers {
     pub fn new(
         timeout: Option<Duration>,
-        on_success: Option<GenericStateUpdate>,
         on_error: Option<GenericStateUpdate>,
         on_timeout: Option<GenericStateUpdate>,
     ) -> Self {
         DefaultHandlers {
             timeout,
-            on_success: on_success.unwrap_or_else(GenericStateUpdate::successful),
             on_error: on_error.unwrap_or_else(GenericStateUpdate::unknown_error),
             on_timeout: on_timeout.unwrap_or_else(GenericStateUpdate::timeout),
         }
@@ -371,7 +368,6 @@ impl Default for DefaultHandlers {
     fn default() -> Self {
         DefaultHandlers {
             timeout: None,
-            on_success: GenericStateUpdate::successful(),
             on_error: GenericStateUpdate::unknown_error(),
             on_timeout: GenericStateUpdate::timeout(),
         }
@@ -664,7 +660,6 @@ on_exit._ = "oops"
             handlers_from_toml_with_defaults("", ""),
             handlers_from_toml(
                 r#"
-on_success = "successful"
 on_timeout = { "status" = "failed", reason = "timeout" }
 on_error = "failed"
 "#
@@ -683,12 +678,11 @@ on_error = "failed"
         );
 
         assert_eq!(
-            handlers_from_toml_with_defaults(r#"on_success = "ok""#, ""),
+            handlers_from_toml_with_defaults(r#"on_error = "broken""#, ""),
             handlers_from_toml(
                 r#"
-on_success = "ok"
 on_timeout = { "status" = "failed", reason = "timeout" }
-on_error = "failed"
+on_error = "broken"
 "#
             ),
         );
diff --git a/crates/core/tedge_api/src/workflow/toml_config.rs b/crates/core/tedge_api/src/workflow/toml_config.rs
@@ -275,16 +275,6 @@ impl TryFrom<(TomlExitHandlers, DefaultHandlers)> for ExitHandlers {
             })
             .collect();
 
-        // Inject defaults if no success state is given either with: on_success, on_exit.0, on_return
-        let on_success = if on_success.is_none()
-            && on_stdout.is_empty()
-            && !on_exit.iter().any(|(i, _, _)| *i == 0)
-        {
-            Some(defaults.on_success)
-        } else {
-            on_success
-        };
-
         // Inject defaults if no error state is given either with: on_error, on_exit._
         let on_error = if on_error.is_none() && wildcard.is_none() {
             Some(defaults.on_error)
@@ -327,7 +317,7 @@ impl TryFrom<(TomlExitHandlers, DefaultHandlers)> for AwaitHandlers {
         let on_success: GenericStateUpdate = handlers
             .on_success
             .map(|u| u.into())
-            .unwrap_or(defaults.on_success);
+            .ok_or(ScriptDefinitionError::MissingOnSuccessHandler)?;
         let on_error = handlers
             .on_error
             .map(|u| u.into())
@@ -372,13 +362,10 @@ impl TryFrom<TomlExitHandlers> for DefaultHandlers {
 
     fn try_from(value: TomlExitHandlers) -> Result<Self, Self::Error> {
         let timeout = value.timeout_second.map(Duration::from_secs);
-        let on_success = value.on_success.map(|u| u.into());
         let on_timeout = value.on_timeout.map(|u| u.into());
         let on_error = value.on_error.map(|u| u.into());
 
-        Ok(DefaultHandlers::new(
-            timeout, on_success, on_error, on_timeout,
-        ))
+        Ok(DefaultHandlers::new(timeout, on_error, on_timeout))
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,9 @@ pub enum ScriptDefinitionError {`
`37`	`37`
`38`	`38`	`#[error("Invalid exit code range '{from}-{to}' as {from}>{to}")]`
`39`	`39`	`IncorrectRange { from: u8, to: u8 },`
	`40`	`+`
	`41`	`+ #[error("No handler is provided for 'on_success'")]`
	`42`	`+ MissingOnSuccessHandler,`
`40`	`43`	`}`
`41`	`44`
`42`	`45`	`/// Error related to state excerpt definitions`