swagger-codegen icon indicating copy to clipboard operation
swagger-codegen copied to clipboard

[python] Avoid creating unused ThreadPools

Open minrk opened this issue 7 years ago • 11 comments

PR checklist

  • [x] Read the contribution guidelines.
  • [x] Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • [x] Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • [x] Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Instead, create ApiClient.pool on first request for .pool property.

This avoids spawning n-cpus threads (the default for ThreadPool) at instantiation of every ApiClient, which can be very costly in, for example, container scenarios on nodes with many CPUs where multiple API Clients might be instantiated in one process. kubernetes unconditionally instantiates an ApiClient only for serialization that never makes any requests every time a Watch is created, for example.

The thread count is made configurable and defaults to 1 instead of N-CPUs, which I think makes more sense for the default behavior of a non-blocking API.

minrk avatar Apr 23 '18 10:04 minrk

cc @taxpon @frol @mbohlool @cbornet @kenjones-cisco for Python review

minrk avatar Apr 27 '18 08:04 minrk

LGTM, but it doesn't seem like you did:

Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows.

frol avatar Apr 27 '18 09:04 frol

Sorry, failed to commit it before. Looks like this had been missed a few times, since there are a few more changes in the results than mine.

minrk avatar May 09 '18 06:05 minrk

Had to fix a deserialization test that hadn't been updated since some changes in enums, I think. The failing test passed locally for me, hopefully Travis will find the same.

minrk avatar May 09 '18 20:05 minrk

LGTM

kenjones-cisco avatar May 10 '18 19:05 kenjones-cisco

Test failure appears to have been an intermittent network issue. Anything I can to do help this progress?

minrk avatar Jun 13 '18 11:06 minrk

Resolved conflicts.

minrk avatar Aug 21 '18 09:08 minrk

This blocks on exit the majority of times when more than one API client is created (https://github.com/kubernetes-client/python/issues/411). What needs to be done to get this merged and released so the k8s Python API can be regenerated and fixed?

007 avatar Nov 05 '18 18:11 007

Rebased and rerendered again

minrk avatar Nov 06 '18 10:11 minrk

Happy anniversary #8061 :tada:

007 avatar Apr 27 '19 04:04 007

Was this ever merged?

noamgat avatar Nov 18 '20 11:11 noamgat