timeouts too low for some actions

[sjoerdsimons]

The bollard API only seems to have one general timeout for all of the client actions (2 minutes by default, or user provided when using connect_with_unix directly).

As always, timeouts are awkward (it's rougly impossible to get them right for all situations). But in this case that same timeout is used for all actions. Some you would hope are always fastish (e.g. listing running containers). However some can take a long while e.g. pulling a big image on a slow connection.

I'm unsure what a good solution is here; For now i'm working around it in client code by just setting the timeout to a day... But i guess the options roughly would be:

• Make timeouts more fine-grained (e.g. ability to specify per action/call or seperate api calls in groups depending on expectations)
• Add ability to simply disable timeouts in bollard, moving the responsibility to client code (or even removing timeouts in bollard itself entirely always making it the callers responsibility)

[fussybeaver]

Interesting question, thanks for raising it..

I'm a bit wary about indefinite timeouts - if the registry you're pulling from is down, you might get a hanging process. I'm not convinced that consumers of the library would do the sensible thing and create their own timeouts.

Having more fine-grained timeouts with a sensible default sounds like a great improvement (if someone wants to pick that up).

[Yoric]

I'm currently faced with a bollard-based app that works nicely for me but timeouts on a project colleague's home computer during build (CLI docker build seems to work nicely for him). I believe that his computer is a bit slower, which may explain it.

The ability to remove or customize timeouts would indeed be very useful.

[fussybeaver]

You can configure it by calling connect_with_unix directly https://docs.rs/bollard/0.11.1/bollard/struct.Docker.html#method.connect_with_unix

I'll look into adding a method to change the timeout after instantiation, it doesn't complicated

[Yoric]

What a coincidence :)

[lavrd]

Thank you for adding setting timeout methods! But as @sjoerdsimons said, we need different timeouts for different methods, because request for a list of containers is a fast operation but download 1gb image is not. Can we reopen this issue to add this possibility?

[fussybeaver]

How do you suggest the API should change ? It would be a bit strange to add a parameter or option to each method with a timeout argument.

Maybe with a macro?

[lavrd]

How it can be done with macro?

My thoughts was an adding method to client like with_timeout, but this method doesn't change timeout in client, just affect timeout in current method.

[fussybeaver]

Happy to re-open this for visibility.

If we can get a granular proposal for the API agreed, that might move the needle in getting it implemented.

[lavrd]

I can try to make a draft pr. What do you think?

[fussybeaver]

That sounds great.

[lavrd]

At the moment I don't understand how to do it without Docker struct redesign

1. as create_image method receives self by reference we can't temporary change timeout field
2. to pass custom timeout to create_image also is not very good idea because it requires a lot of useless changes to functions which use process_request

[fussybeaver]

Thanks for giving it a shot.. I thought maybe something simpler like having a function with a callback might work, but in my naive attempt it was having issues with the borrow checker.

[lavrd]

The problem with big sized images can be solved like this:

let ci = client
        .create_image(
            Some(CreateImageOptions {
                from_image,
                ..Default::default()
            }),
            None,
            credentials,
        )
        .try_collect::<Vec<_>>();
tokio::time::timeout(Duration::from_secs(1), ci).await.unwrap().unwrap();

So, maybe timeouts per function are not necessary.