Skip to content
This repository was archived by the owner on Mar 13, 2022. It is now read-only.

Use a higher value for connecion_pool_maxsize. #29

Merged
merged 1 commit into from
Aug 23, 2017

Conversation

pokoli
Copy link

@pokoli pokoli commented Aug 22, 2017

Fixes #26

@mbohlool
Copy link
Contributor

This looks good to me. I will let @yuvipanda test and confirm this before merging. Thanks @pokoli.

@codecov-io
Copy link

codecov-io commented Aug 22, 2017

Codecov Report

Merging #29 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #29   +/-   ##
=======================================
  Coverage   93.93%   93.93%           
=======================================
  Files          11       11           
  Lines         824      824           
=======================================
  Hits          774      774           
  Misses         50       50

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4127e1a...c38bc4d. Read the comment docs.

Copy link
Contributor

@yuvipanda yuvipanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment about additional comments in the code, otherwise +1 yay!

# per pool. urllib3 uses 1 connection as default value, but this is
# not the best value when you are making a lot of possibly parallel
# requests to the same host, which is often the case here.
# cpu_count * 5 is used as default value to increase performance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a note saying we're doing this because this is the default for ThreadPoolExecutor?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, thanks.

@mbohlool mbohlool merged commit 84d1284 into kubernetes-client:master Aug 23, 2017
@pokoli
Copy link
Author

pokoli commented Aug 23, 2017

Thanks for the review guys!

@pokoli pokoli deleted the pool_maxsize branch August 23, 2017 11:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants