From 232be301c3bf60fc3a351f13e8b251f86477d35c Mon Sep 17 00:00:00 2001 From: Sibi Prabakaran Date: Thu, 25 Jul 2024 19:30:35 +0530 Subject: [PATCH 1/4] Make slack_webhook as non optional parameter Initial the CLI argument slack_webhook was optional, even though other options related to notificatin were required to be passed (Thanks to Kirill for raising this point to me): - app name - app version - notification context I initially made the slack_webhook option to convey the fact that the notification system is pluggable and you could instead optionally integrate other providers like Discord or Microsoft teams. Making it as non optional would indicate that this tool is tied to slack. I discovered that this problem is solved by clap's ArgGroup, so I have converted the solution to that. This also makes sure that we pass atleast one parameter of the NotifyHook struct. --- src/cli.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index dfec7a4..110686c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -25,9 +25,9 @@ pub(crate) struct Cli { /// Seconds to wait for output before killing the task #[arg(long)] pub(crate) task_output_timeout: Option, - /// Slack Webhook for notification - #[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")] - pub(crate) slack_webhook: Option, + /// Notification hook + #[command(flatten)] + pub(crate) notification_hook: NotifyHook, /// Application description #[arg(long)] pub(crate) app_description: String, @@ -54,6 +54,13 @@ pub(crate) struct Cli { pub(crate) output_lines: usize, } +#[derive(clap::Args)] +#[group(required = true, multiple = false)] +pub(crate) struct NotifyHook { + #[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")] + pub(crate) slack_webhook: Option, +} + #[derive(Debug)] enum MainMessage { Error(anyhow::Error), @@ -184,7 +191,7 @@ impl Cli { match res { Ok(()) => Ok(()), Err(e) => { - if let Some(slack_webhook) = self.slack_webhook { + if let Some(slack_webhook) = self.notification_hook.slack_webhook { let slack_app = SlackApp::new( slack_webhook, self.notification_context, From 0ea46afc62906ce80a75e7b023091c35a6e589ab Mon Sep 17 00:00:00 2001 From: Sibi Prabakaran Date: Thu, 25 Jul 2024 20:21:41 +0530 Subject: [PATCH 2/4] Remove Option from slack_webhook --- src/cli.rs | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 110686c..7193485 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -58,7 +58,7 @@ pub(crate) struct Cli { #[group(required = true, multiple = false)] pub(crate) struct NotifyHook { #[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")] - pub(crate) slack_webhook: Option, + pub(crate) slack_webhook: Url, } #[derive(Debug)] @@ -191,23 +191,21 @@ impl Cli { match res { Ok(()) => Ok(()), Err(e) => { - if let Some(slack_webhook) = self.notification_hook.slack_webhook { - let slack_app = SlackApp::new( - slack_webhook, - self.notification_context, - self.app_description, - self.app_version, - self.image_url, - ); - let mut msg = String::new(); - for line in &*recent_output.lock() { - msg.push_str(line); - msg.push('\n'); - } - let result = slack_app.send_notification(&e, &msg); - if let Err(err) = result { - eprintln!("Slack notification failed: {err:?}"); - } + let slack_app = SlackApp::new( + self.notification_hook.slack_webhook, + self.notification_context, + self.app_description, + self.app_version, + self.image_url, + ); + let mut msg = String::new(); + for line in &*recent_output.lock() { + msg.push_str(line); + msg.push('\n'); + } + let result = slack_app.send_notification(&e, &msg); + if let Err(err) = result { + eprintln!("Slack notification failed: {err:?}"); } Err(e) } From b9e9bbbcaab87d1c0ecb716d67d1d6bd89a2633f Mon Sep 17 00:00:00 2001 From: Sibi Prabakaran Date: Thu, 25 Jul 2024 20:38:22 +0530 Subject: [PATCH 3/4] Use slack_webhook directly --- src/cli.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 7193485..ac1d56f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -25,9 +25,9 @@ pub(crate) struct Cli { /// Seconds to wait for output before killing the task #[arg(long)] pub(crate) task_output_timeout: Option, - /// Notification hook - #[command(flatten)] - pub(crate) notification_hook: NotifyHook, + /// Slack Webhook for notification + #[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")] + pub(crate) slack_webhook: Url, /// Application description #[arg(long)] pub(crate) app_description: String, @@ -192,7 +192,7 @@ impl Cli { Ok(()) => Ok(()), Err(e) => { let slack_app = SlackApp::new( - self.notification_hook.slack_webhook, + self.slack_webhook, self.notification_context, self.app_description, self.app_version, From 8f2d8f76a64e00dd60dde484274a17c4bef5261e Mon Sep 17 00:00:00 2001 From: Sibi Prabakaran Date: Thu, 25 Jul 2024 20:39:25 +0530 Subject: [PATCH 4/4] Remove NotifyHook --- src/cli.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index ac1d56f..59405bf 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -54,13 +54,6 @@ pub(crate) struct Cli { pub(crate) output_lines: usize, } -#[derive(clap::Args)] -#[group(required = true, multiple = false)] -pub(crate) struct NotifyHook { - #[arg(long, value_parser(Url::from_str), env = "HEALTH_CHECK_SLACK_WEBHOOK")] - pub(crate) slack_webhook: Url, -} - #[derive(Debug)] enum MainMessage { Error(anyhow::Error),