Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker Agent: Passing custom parameters when creating docker.APIClient #4220

Closed
hiendang opened this issue Mar 8, 2021 · 4 comments · Fixed by #4232
Closed

Docker Agent: Passing custom parameters when creating docker.APIClient #4220

hiendang opened this issue Mar 8, 2021 · 4 comments · Fixed by #4232
Labels
enhancement An improvement of an existing feature

Comments

@hiendang
Copy link

hiendang commented Mar 8, 2021

Current behavior

prefect docker agent is using default parameters to create an instance of docker.APIClient. This makes it impossible to customize the docker client. Things such as to increase the timeout of docker client cannot be done.

https://github.com/PrefectHQ/prefect/blob/master/src/prefect/agent/docker/agent.py#L186

Proposed behavior

We should have a way to pass custom parameters to when creating an instance of docker.APIClient.
Either as a dictionary or as a json config file

Example

When the somehow get overloaded, docker daemon failed to create a container within 60 seconds, docker.APIClient will then return a timeout error similar to docker/compose#3927. In my case increase the timeout from 60 to 120 seconds would mitigate this problem.

@hiendang hiendang added the enhancement An improvement of an existing feature label Mar 8, 2021
@jcrist
Copy link

jcrist commented Mar 8, 2021

Would exposing only the timeout be sufficient. Exposing the full kwargs options through the CLI is tricky to support in an ergonomic way, exposing individual parameters is much simpler.

I'd write this as

$ prefect agent docker start --docker-client-timeout 120

@hiendang
Copy link
Author

hiendang commented Mar 9, 2021

yes I agreed, it would be sufficient in this case. Passing full kwargs would be messy and maybe only timeout parameter is needed at the end

@zanieb
Copy link
Contributor

zanieb commented Mar 9, 2021

It would also be nice to be able to set credstore_env but I can't remember where the thread was where a user needed it.

@jcrist
Copy link

jcrist commented Mar 9, 2021

It would also be nice to be able to set credstore_env but I can't remember where the thread was where a user needed it.

I'm going to leave that one out for now, if a user needs it they can elevate it to an issue later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants