From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 6C66E1FF137 for ; Tue, 31 Mar 2026 14:06:07 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E14E81AE35; Tue, 31 Mar 2026 14:06:34 +0200 (CEST) Message-ID: <5cb3e221-fb33-493a-bdf5-1f8582492abb@proxmox.com> Date: Tue, 31 Mar 2026 14:05:48 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH yew-comp 1/3] task (viewer): add task download button To: Maximiliano Sandoval References: <20260330125320.507302-1-d.csapak@proxmox.com> <20260330125320.507302-2-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774958702630 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.457 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: YSPFAP6572EFVLQCWTXGVJKQKSTLLF3R X-Message-ID-Hash: YSPFAP6572EFVLQCWTXGVJKQKSTLLF3R X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: yew-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Yew framework devel list at Proxmox List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 3/31/26 1:13 PM, Maximiliano Sandoval wrote: > Dominik Csapak writes: > >> similar to what we already have in the ExtJS gui. Currently the default >> is to construct a data URL from the downloaded json in the UI, since not >> all APIs support the 'download=1' parameter (and some can't be easily >> extended). >> >> The download api can be enabled with a setting though, so if at some >> point all consumers use that, we can drop the compatibility code path. >> >> Adds also a small helper to create a temporary 'a' element to click. >> >> Signed-off-by: Dominik Csapak >> --- >> Longer explanation which api calls do not support the 'download' >> parameter: on PDM, we tunnel the api call to the remote, but this is >> autogenerated code that expects json output. With the 'download' >> parameter, the content type gets set to text/plain, which confuses the >> client. >> >> To fix it, we'd either have to rewrite the auto-code generation to >> handle this case, or expose the underlying client and doit manually, or >> rewrite the remote task api call to to something similar what we do here >> in the ui, collecting json output and returning a final log txt file. >> >> Since having this download button is useful anyway, and implementing any >> of these solutions would take way longer than doing this here, I opted >> for doing that now, and fixing the backend later. >> >> src/lib.rs | 21 ++++++++ >> src/log_view.rs | 7 +-- >> src/task_viewer.rs | 118 +++++++++++++++++++++++++++++++++++++++++---- >> src/tasks.rs | 6 +++ >> 4 files changed, 140 insertions(+), 12 deletions(-) >> >> diff --git a/src/lib.rs b/src/lib.rs >> index 62aa571..65dae68 100644 >> --- a/src/lib.rs >> +++ b/src/lib.rs >> @@ -234,6 +234,8 @@ pub mod utils; >> mod xtermjs; >> pub use xtermjs::{ConsoleType, ProxmoxXTermJs, XTermJs}; >> >> +use anyhow::{format_err, Error}; >> + >> use pwt::gettext_noop; >> use pwt::state::{LanguageInfo, TextDirection}; >> >> @@ -271,6 +273,25 @@ mod panic_wrapper { >> } >> } >> >> +/// Helper to download the given 'source' url via a simulated click on a hidden 'a' element. >> +pub fn download_as_file(source: &str, file_name: &str) -> Result<(), Error> { >> + let el = gloo_utils::document() >> + .create_element("a") >> + .map_err(|_| format_err!("unable to create element 'a'"))?; >> + let html = el >> + .dyn_into::() > > Is it possible a `use wasm_bindgen::JsCast;` > > is missing in this function or file? This does not compile. > yes that's possible. Not sure why, when I compile pdm with this as path dep override, it compiles with no issues, but a 'make deb' fails here... > >> + .map_err(|_| format_err!("unable to cast to html element"))?; >> + html.set_attribute("href", source) >> + .map_err(|_| format_err!("unable to set href attribute"))?; >> + html.set_attribute("target", "_blank") >> + .map_err(|_| format_err!("unable to set target attribute"))?; >> + html.set_attribute("download", file_name) >> + .map_err(|_| format_err!("unable to set download attribute"))?; >> + html.click(); >> + html.remove(); >> + Ok(()) >> +} >> + >> pub fn store_csrf_token(crsf_token: &str) { >> if let Some(store) = pwt::state::session_storage() { >> if store.set_item("CSRFToken", crsf_token).is_err() { >> diff --git a/src/log_view.rs b/src/log_view.rs >> index 20f8f77..46bc758 100644 >> --- a/src/log_view.rs >> +++ b/src/log_view.rs >> @@ -37,10 +37,11 @@ const MAX_PHYSICAL: f64 = 17_000_000.0; >> const DEFAULT_LINE_HEIGHT: u64 = 18; >> >> #[derive(Deserialize)] >> -struct LogEntry { >> +/// a single Log entry line >> +pub struct LogEntry { >> #[allow(dead_code)] >> - n: u64, >> - t: String, >> + pub n: u64, >> + pub t: String, >> } >> >> pub struct LogPage { >> diff --git a/src/task_viewer.rs b/src/task_viewer.rs >> index e78454e..decf721 100644 >> --- a/src/task_viewer.rs >> +++ b/src/task_viewer.rs >> @@ -1,6 +1,6 @@ >> use std::rc::Rc; >> >> -use serde_json::Value; >> +use serde_json::{json, Value}; >> >> use gloo_timers::callback::Timeout; >> >> @@ -9,15 +9,19 @@ use yew::prelude::*; >> use yew::virtual_dom::{Key, VComp, VNode}; >> >> use pwt::state::Loader; >> -use pwt::widget::{Button, Column, Dialog, TabBarItem, TabPanel, Toolbar}; >> +use pwt::widget::{AlertDialog, Button, Column, Container, Dialog, TabBarItem, TabPanel, Toolbar}; >> use pwt::{prelude::*, AsyncPool}; >> >> +use crate::common_api_types::ProxmoxUpid; >> +use crate::log_view::LogEntry; >> use crate::percent_encoding::percent_encode_component; >> use crate::utils::{format_duration_human, format_upid, render_epoch}; >> -use crate::{KVGrid, KVGridRow, LogView}; >> +use crate::{download_as_file, KVGrid, KVGridRow, LogView}; >> >> use pwt_macros::builder; >> >> +use proxmox_time::epoch_to_rfc3339_utc; >> + >> #[derive(Properties, PartialEq, Clone)] >> #[builder] >> pub struct TaskViewer { >> @@ -39,6 +43,12 @@ pub struct TaskViewer { >> #[builder(IntoPropValue, into_prop_value)] >> /// The base url for >> pub base_url: AttrValue, >> + >> + #[prop_or(false)] >> + #[builder(IntoPropValue, into_prop_value)] >> + /// Use the 'download' API parameter for downloading task logs. >> + /// By default the task log will be downloaded as json, and converted to a data URL. >> + pub download_api: bool, >> } >> >> impl TaskViewer { >> @@ -55,6 +65,7 @@ pub enum Msg { >> DataChange, >> Reload, >> StopTask, >> + DownloadErr(Option), >> } >> >> pub struct PwtTaskViewer { >> @@ -63,6 +74,7 @@ pub struct PwtTaskViewer { >> active: bool, >> endtime: Option, >> async_pool: AsyncPool, >> + download_err: Option, >> } >> >> impl Component for PwtTaskViewer { >> @@ -99,6 +111,7 @@ impl Component for PwtTaskViewer { >> active: props.endtime.is_none(), >> endtime: props.endtime, >> async_pool: AsyncPool::new(), >> + download_err: None, >> } >> } >> >> @@ -138,12 +151,23 @@ impl Component for PwtTaskViewer { >> } >> true >> } >> + Msg::DownloadErr(err) => { >> + self.download_err = err; >> + true >> + } >> } >> } >> >> fn view(&self, ctx: &Context) -> Html { >> let props = ctx.props(); >> >> + if let Some(err) = &self.download_err { >> + return AlertDialog::new(err) >> + .title(tr!("Download Error")) >> + .on_close(ctx.link().callback(|_| Msg::DownloadErr(None))) >> + .into(); >> + } >> + >> let panel = self.loader.render(|data| { >> TabPanel::new() >> .class("pwt-flex-fit") >> @@ -273,18 +297,94 @@ impl PwtTaskViewer { >> let active = self.active; >> let link = ctx.link(); >> >> - let toolbar = Toolbar::new().class("pwt-border-bottom").with_child( >> - Button::new(tr!("Stop")) >> - .disabled(!active) >> - .onclick(link.callback(|_| Msg::StopTask)), >> - ); >> - >> let url = format!( >> "{}/{}/log", >> props.base_url, >> percent_encode_component(&task_id), >> ); >> >> + let filename = match props.task_id.parse::() { >> + Ok(upid) => { >> + format!( >> + "task-{}-{}-{}.log", >> + upid.node, >> + upid.worker_type, >> + epoch_to_rfc3339_utc(upid.starttime).unwrap_or(upid.starttime.to_string()) >> + ) >> + } >> + Err(_) => format!("task-{}.log", props.task_id), >> + }; >> + >> + let download_btn: Html = if props.download_api { >> + Container::from_tag("a") >> + .attribute("disabled", active.then_some("")) >> + .attribute("target", "_blank") >> + .attribute("href", format!("/api2/extjs{url}?download=1")) >> + .attribute("filename", filename) >> + .with_child( >> + Button::new(tr!("Download")) >> + .disabled(active) >> + .icon_class("fa fa-download"), >> + ) >> + .into() >> + } else { >> + Button::new(tr!("Download")) >> + .icon_class("fa fa-download") >> + .disabled(active) >> + .on_activate({ >> + let url = url.clone(); >> + let link = link.clone(); >> + let async_pool = self.async_pool.clone(); >> + move |_| { >> + let url = url.clone(); >> + let filename = filename.clone(); >> + let link = link.clone(); >> + async_pool.spawn(async move { >> + let param = json!({ >> + "start": 0, >> + "limit": 0, >> + }); >> + let resp = >> + crate::http_get_full::>(url, Some(param)).await; >> + match resp { >> + Ok(resp) => { >> + let string = resp >> + .data >> + .into_iter() >> + .map(|entry| entry.t) >> + .collect::>() >> + .join("\n"); >> + >> + if let Err(err) = download_as_file( >> + &format!( >> + "data:text/plain;charset=utf-8,{}", >> + percent_encode_component(&string) >> + ), >> + &filename, >> + ) { >> + link.send_message(Msg::DownloadErr(Some(err.to_string()))) >> + } >> + } >> + Err(err) => { >> + link.send_message(Msg::DownloadErr(Some(err.to_string()))) >> + } >> + } >> + }); >> + } >> + }) >> + .into() >> + }; >> + >> + let toolbar = Toolbar::new() >> + .class("pwt-border-bottom") >> + .with_child( >> + Button::new(tr!("Stop")) >> + .disabled(!active) >> + .onclick(link.callback(|_| Msg::StopTask)), >> + ) >> + .with_flex_spacer() >> + .with_child(download_btn); >> + >> Column::new() >> .class("pwt-flex-fit") >> .with_child(toolbar) >> diff --git a/src/tasks.rs b/src/tasks.rs >> index b49de98..fb3ed39 100644 >> --- a/src/tasks.rs >> +++ b/src/tasks.rs >> @@ -78,6 +78,11 @@ pub struct Tasks { >> #[prop_or_default] >> /// An optional column configuration that overwrites the default one. >> pub columns: Option>>>, >> + >> + #[prop_or(false)] >> + #[builder(IntoPropValue, into_prop_value)] >> + /// Use the 'download' API parameter for downloading task logs. >> + pub download_api: bool, >> } >> >> impl Default for Tasks { >> @@ -493,6 +498,7 @@ impl LoadableComponent for ProxmoxTasks { >> match view_state { >> ViewDialog::TaskViewer => { >> let mut dialog = TaskViewer::new(&*selected_key) >> + .download_api(props.download_api) >> .endtime(selected_item.endtime) >> .on_close(ctx.link().change_view_callback(|_| None)); >> if let Some(base_url) = &props.base_url { >