From b3f1ed596c92f07c37b9a00609ec214f5ce5a905 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 22 Dec 2024 17:35:16 +0000 Subject: [PATCH] fix: stop zombie actors Some actors had been set up so that they only stopped when all their children stopped. The detection for this was triggered by each child as it stopped. However, some actors didn't have any children, so were never triggered to stop. They now check after starting any children if they should simply stop there and then. --- src/import/attachment.rs | 9 +++------ src/import/card.rs | 34 ++++++++++++++++++++++++++-------- src/import/stack.rs | 21 ++++++++++++++++----- src/import/stacks.rs | 17 ++++++++++++----- src/lib.rs | 3 ++- src/trello/client.rs | 2 +- src/trello/model/attachment.rs | 23 ++++++++++++++++++----- 7 files changed, 78 insertions(+), 31 deletions(-) diff --git a/src/import/attachment.rs b/src/import/attachment.rs index 78ee044..9b628ab 100644 --- a/src/import/attachment.rs +++ b/src/import/attachment.rs @@ -6,10 +6,7 @@ use kameo::{mailbox::unbounded::UnboundedMailbox, Actor}; use crate::{ nextcloud::model::{NextcloudBoardId, NextcloudCardId, NextcloudStackId}, on_actor_start, p, - trello::model::{ - attachment::{TrelloAttachment, TrelloAttachmentId}, - card::TrelloCardId, - }, + trello::model::{attachment::TrelloAttachment, card::TrelloCardId}, FullCtx, }; @@ -55,8 +52,8 @@ impl Actor for ImportAttachmentActor { .trello_client() .save_attachment( &this.trello_card_id, - &TrelloAttachmentId::new(&this.trello_attachment.id), - Some(&PathBuf::from(&this.trello_attachment.id)), + &this.trello_attachment.id, + Some(&PathBuf::from(&(*this.trello_attachment.id))), ) .await?; let attachment_file = this.ctx.fs.file(&attachment_path); diff --git a/src/import/card.rs b/src/import/card.rs index 2d43fa7..a624e38 100644 --- a/src/import/card.rs +++ b/src/import/card.rs @@ -15,7 +15,10 @@ use crate::{ model::{NextcloudBoardId, NextcloudCardDescription, NextcloudCardTitle, NextcloudStackId}, }, on_actor_link_died, on_actor_start, p, spawn_in_thread, - trello::{client::TrelloClient, model::card::TrelloShortCard}, + trello::{ + client::TrelloClient, + model::{attachment::TrelloAttachmentName, card::TrelloShortCard, label::TrelloLabelName}, + }, FullCtx, }; @@ -26,8 +29,8 @@ pub(crate) struct ImportCardActor { nextcloud_stack_id: NextcloudStackId, labels_actor_ref: ActorRef, - labels_children: HashMap>, - attachments_children: HashMap>, + labels_children: HashMap)>, + attachments_children: HashMap)>, } impl ImportCardActor { pub(crate) fn new( @@ -78,6 +81,7 @@ impl Actor for ImportCardActor { let mut labels = vec![]; std::mem::swap(&mut this.trello_card.labels, &mut labels); for trello_label in labels.into_iter() { + let trello_label_name = trello_label.name.clone(); let child = spawn_in_thread!( actor_ref, ImportLabelActor::new( @@ -89,7 +93,8 @@ impl Actor for ImportCardActor { this.labels_actor_ref.clone(), ) ); - this.labels_children.insert(child.id(), child.clone()); + this.labels_children + .insert(child.id(), (trello_label_name, child.clone())); } let attachments = trello_client @@ -98,6 +103,7 @@ impl Actor for ImportCardActor { .result? .attachments; for trello_attachment in attachments.into_iter() { + let trello_attachment_name = trello_attachment.name.clone(); let child = spawn_in_thread!( actor_ref, ImportAttachmentActor::new( @@ -109,23 +115,35 @@ impl Actor for ImportCardActor { trello_attachment, ) ); - this.attachments_children.insert(child.id(), child.clone()); + this.attachments_children + .insert(child.id(), (trello_attachment_name, child.clone())); } - Ok(()) + if this.labels_children.is_empty() && this.attachments_children.is_empty() { + Ok(actor_ref.stop_gracefully().await?) + } else { + Ok(()) + } }); on_actor_link_died!(this, actor_ref, id, reason, { match reason { ActorStopReason::Normal => { - this.labels_children.remove(&id); - this.attachments_children.remove(&id); + let label = this.labels_children.remove(&id); + let attachment = this.attachments_children.remove(&id); + tracing::debug!( + ?id, + ?label, + ?attachment, + "card (label|attachment) child stopped" + ); } _ => { return Ok(Some(reason)); } } + tracing::debug!(?this.labels_children, ?this.attachments_children, "card children"); if this.labels_children.is_empty() && this.attachments_children.is_empty() { return Ok(Some(ActorStopReason::Normal)); } diff --git a/src/import/stack.rs b/src/import/stack.rs index ef1757b..34222fc 100644 --- a/src/import/stack.rs +++ b/src/import/stack.rs @@ -14,7 +14,10 @@ use crate::{ on_actor_link_died, on_actor_start, spawn_in_thread, trello::{ client::TrelloClient, - model::list::{TrelloList, TrelloListId}, + model::{ + card::TrelloCardName, + list::{TrelloList, TrelloListId}, + }, }, FullCtx, }; @@ -25,7 +28,7 @@ pub(super) struct ImportStackActor { nextcloud_board_id: NextcloudBoardId, nextcloud_stack_id: NextcloudStackId, labels_actor_ref: ActorRef, - children: HashMap>, + children: HashMap)>, } impl ImportStackActor { pub(crate) fn new( @@ -60,6 +63,7 @@ impl Actor for ImportStackActor { // - for each card in the trello stack for trello_card in trello_cards.into_iter() { + let trello_card_name = trello_card.name.clone(); let child: ActorRef = spawn_in_thread!( actor_ref.clone(), ImportCardActor::new( @@ -70,20 +74,27 @@ impl Actor for ImportStackActor { this.labels_actor_ref.clone(), ) ); - this.children.insert(child.id(), child.clone()); + this.children + .insert(child.id(), (trello_card_name, child.clone())); + } + if this.children.is_empty() { + Ok(actor_ref.stop_gracefully().await?) + } else { + Ok(()) } - Ok(()) }); on_actor_link_died!(this, actor_ref, id, reason, { match reason { ActorStopReason::Normal => { - this.children.remove(&id); + let stopped = this.children.remove(&id); + tracing::debug!(?id, ?stopped, "stack card child stopped"); } _ => { return Ok(Some(reason)); } } + tracing::debug!(children = ?this.children, "stack children"); if this.children.is_empty() { return Ok(Some(ActorStopReason::Normal)); } diff --git a/src/import/stacks.rs b/src/import/stacks.rs index 0f217b5..0d917dd 100644 --- a/src/import/stacks.rs +++ b/src/import/stacks.rs @@ -15,7 +15,7 @@ use crate::{ model::{NextcloudBoardId, Stack}, }, on_actor_link_died, on_actor_start, p, spawn_in_thread, - trello::model::list::TrelloList, + trello::model::list::{TrelloList, TrelloListName}, FullCtx, }; @@ -58,7 +58,7 @@ pub(super) struct ImportStacksActor { nextcloud_board_id: NextcloudBoardId, labels_actor_ref: ActorRef, - children: HashMap>, + children: HashMap)>, } impl ImportStacksActor { pub(super) fn new( @@ -128,20 +128,27 @@ impl Actor for ImportStacksActor { this.labels_actor_ref.clone(), ) ); - this.children.insert(child.id(), child); + this.children + .insert(child.id(), (selected_trello_stack.name.clone(), child)); + } + if this.children.is_empty() { + Ok(actor_ref.stop_gracefully().await?) + } else { + Ok(()) } - Ok(()) }); on_actor_link_died!(this, actor_ref, id, reason, { match reason { ActorStopReason::Normal => { - this.children.remove(&id); + let stopped = this.children.remove(&id); + tracing::debug!(?id, ?stopped, "stacks stack child stopped"); } _ => { return Ok(Some(reason)); } } + tracing::debug!(?this.children, "stacks children"); if this.children.is_empty() { return Ok(Some(ActorStopReason::Normal)); } diff --git a/src/lib.rs b/src/lib.rs index 4c212df..75cca03 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -108,10 +108,11 @@ pub async fn run(ctx: &Ctx, commands: &Commands) -> color_eyre::Result<()> { tracing::subscriber::set_global_default( tracing_subscriber::FmtSubscriber::builder() .with_max_level(tracing::Level::TRACE) + .with_env_filter("trace,hyper_util=info,kxio=info") .finish(), )?; - tracing::info!("ready"); } + tracing::info!("ready"); let cfg = AppConfig::load(ctx); match cfg { diff --git a/src/trello/client.rs b/src/trello/client.rs index c674a4a..9630f0c 100644 --- a/src/trello/client.rs +++ b/src/trello/client.rs @@ -77,7 +77,7 @@ impl<'ctx> TrelloClient<'ctx> { let url = attachment.url; let file_name = file_name .cloned() - .unwrap_or_else(|| PathBuf::from(attachment.file_name)); + .unwrap_or_else(|| PathBuf::from((*attachment.file_name).clone())); let file_name = self.ctx.fs.base().join(file_name); let resp = with_exponential_backoff!( &self.ctx, diff --git a/src/trello/model/attachment.rs b/src/trello/model/attachment.rs index 57ae7e3..8c75307 100644 --- a/src/trello/model/attachment.rs +++ b/src/trello/model/attachment.rs @@ -5,12 +5,25 @@ use serde::Deserialize; use crate::newtype; newtype!(TrelloAttachmentId, String, Display, "Card Attachment ID"); +newtype!( + TrelloAttachmentName, + String, + Display, + "Card Attachment name" +); +newtype!(TrelloAttachmentUrl, String, Display, "Card Attachment url"); +newtype!( + TrelloAttachmentFilename, + String, + Display, + "Card Attachment filename" +); -#[derive(Clone, Debug, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] pub(crate) struct TrelloAttachment { - pub(crate) id: String, // "5abbe4b7ddc1b351ef961414", - pub(crate) name: String, //"Deprecation Extension Notice", - pub(crate) url: String, //"https://admin.typeform.com/form/RzExEM/share#/link", + pub(crate) id: TrelloAttachmentId, // "5abbe4b7ddc1b351ef961414", + pub(crate) name: TrelloAttachmentName, //"Deprecation Extension Notice", + pub(crate) url: TrelloAttachmentUrl, //"https://admin.typeform.com/form/RzExEM/share#/link", #[serde(rename = "fileName")] - pub(crate) file_name: String, + pub(crate) file_name: TrelloAttachmentFilename, }