Re: [pdm-devel] [PATCH datacenter-manager 1/7] server: generic multi-client wrapper

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Feb 11, 2025 at 03:50:24PM +0100, Lukas Wagner wrote:
> 
> 
> On  2025-02-04 10:55, Wolfgang Bumiller wrote:
> > +
> > +// doing this via a generic method is currently tedious as it requires an extra helper trait to
> > +// declare the flow of the lifetime in the `self.request` vs `self.streaming_request` function from
> > +// its input to its generic output future... and then you run into borrow-checker limitations...
> > +macro_rules! try_request {
> > +    ($self:expr, $method:expr, $path_and_query:expr, $params:expr, $how:ident) => {
> > +        let params = $params.map(serde_json::to_value);
> > +        Box::pin(async move {
> > +            let params = params
> > +                .transpose()
> > +                .map_err(|err| proxmox_client::Error::Anyhow(err.into()))?;
> > +
> > +            let mut last_err = None;
> > +            let mut timed_out = false;
> > +            // The iterator in use here will automatically mark a client as faulty if we move on to
> > +            // the `next()` one.
> > +            for client in $self.try_clients() {
> > +                if let Some(err) = last_err.take() {
> > +                    log::error!("API client error, trying another remote - {err:?}");
> > +                }
> > +                if timed_out {
> > +                    timed_out = false;
> > +                    log::error!("API client timed out, trying another remote");
> > +                }
> > +
> > +                let request = client.$how($method.clone(), $path_and_query, params.as_ref());
> > +                match tokio::time::timeout($self.timeout, request).await {
> > +                    Ok(Err(proxmox_client::Error::Client(err))) => {
> > +                        last_err = Some(err);
> > +                    }
> > +                    Ok(result) => return result,
> > +                    Err(_) => {
> > +                        timed_out = true;
> > +                    }
> > +                }
> > +            }
> 
> maybe add another 
> 
> if let Some(err) = last_err {
>     log::error!("... {err} ...");
> }
> 
> so that the actual `err` from the last client you tried is also logged?
> Also if the *last* client to try times out I guess we would like to log that as well?

Good point, will include in a v2.


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel