From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 040921FF141 for ; Mon, 30 Mar 2026 15:36:12 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 601E8316B5; Mon, 30 Mar 2026 15:36:38 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 30 Mar 2026 15:36:33 +0200 Message-Id: Subject: Re: [PATCH yew-comp 1/3] task (viewer): add task download button To: "Dominik Csapak" X-Mailer: aerc 0.20.0 References: <20260330125320.507302-1-d.csapak@proxmox.com> <20260330125320.507302-2-d.csapak@proxmox.com> In-Reply-To: <20260330125320.507302-2-d.csapak@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1774877738952 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.378 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: S4WWF37NGQMG3NLLTYM2EMTEGYUZJJQ5 X-Message-ID-Hash: S4WWF37NGQMG3NLLTYM2EMTEGYUZJJQ5 X-MailFrom: s.sterz@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 Mon Mar 30, 2026 at 2:53 PM CEST, Dominik Csapak wrote: > 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=3D1' 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<(), Err= or> { > + let el =3D gloo_utils::document() > + .create_element("a") > + .map_err(|_| format_err!("unable to create element 'a'"))?; > + let html =3D el > + .dyn_into::() > + .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) =3D 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 =3D 17_000_000.0; > const DEFAULT_LINE_HEIGHT: u64 =3D 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, TabBar= Item, 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 converte= d 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) =3D> { > + self.download_err =3D err; > + true > + } > } > } > > fn view(&self, ctx: &Context) -> Html { > let props =3D ctx.props(); > > + if let Some(err) =3D &self.download_err { > + return AlertDialog::new(err) > + .title(tr!("Download Error")) > + .on_close(ctx.link().callback(|_| Msg::DownloadErr(None)= )) > + .into(); > + } > + > let panel =3D self.loader.render(|data| { > TabPanel::new() > .class("pwt-flex-fit") > @@ -273,18 +297,94 @@ impl PwtTaskViewer { > let active =3D self.active; > let link =3D ctx.link(); > > - let toolbar =3D Toolbar::new().class("pwt-border-bottom").with_c= hild( > - Button::new(tr!("Stop")) > - .disabled(!active) > - .onclick(link.callback(|_| Msg::StopTask)), > - ); > - > let url =3D format!( > "{}/{}/log", > props.base_url, > percent_encode_component(&task_id), > ); > > + let filename =3D match props.task_id.parse::() { > + Ok(upid) =3D> { > + format!( > + "task-{}-{}-{}.log", > + upid.node, > + upid.worker_type, > + epoch_to_rfc3339_utc(upid.starttime).unwrap_or(upid.= starttime.to_string()) > + ) > + } > + Err(_) =3D> format!("task-{}.log", props.task_id), > + }; > + > + let download_btn: Html =3D if props.download_api { > + Container::from_tag("a") > + .attribute("disabled", active.then_some("")) > + .attribute("target", "_blank") > + .attribute("href", format!("/api2/extjs{url}?download=3D= 1")) > + .attribute("filename", filename) > + .with_child( > + Button::new(tr!("Download")) > + .disabled(active) > + .icon_class("fa fa-download"), > + ) > + .into() is there a reason you construct a new `` element as a container here instead of using the `download_as_file` helper in an `on_activate` callback for the button? > + } else { > + Button::new(tr!("Download")) > + .icon_class("fa fa-download") > + .disabled(active) > + .on_activate({ > + let url =3D url.clone(); > + let link =3D link.clone(); > + let async_pool =3D self.async_pool.clone(); > + move |_| { > + let url =3D url.clone(); > + let filename =3D filename.clone(); > + let link =3D link.clone(); > + async_pool.spawn(async move { > + let param =3D json!({ > + "start": 0, > + "limit": 0, > + }); > + let resp =3D > + crate::http_get_full::>(ur= l, Some(param)).await; > + match resp { > + Ok(resp) =3D> { > + let string =3D resp > + .data > + .into_iter() > + .map(|entry| entry.t) > + .collect::>() > + .join("\n"); > + > + if let Err(err) =3D download_as_file= ( > + &format!( > + "data:text/plain;charset=3Du= tf-8,{}", > + percent_encode_component(&st= ring) > + ), > + &filename, > + ) { > + link.send_message(Msg::DownloadE= rr(Some(err.to_string()))) > + } > + } > + Err(err) =3D> { > + link.send_message(Msg::DownloadErr(S= ome(err.to_string()))) > + } > + } > + }); > + } > + }) > + .into() > + }; > + > + let toolbar =3D 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 =3D> { > let mut dialog =3D 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) =3D &props.base_url {