added basic asyncio support#2547
Conversation
k8s-ci-robot
added
release-note
|
/assign @roycaihw |
|
👏 |
roycaihw
left a comment
roycaihw
left a comment
There was a problem hiding this comment.
One minor comment regarding README. LGTM otherwise.
| + # There is no hard-limit defined by k8s, but the etcd default | ||
| + # maximum request size is 1.5MiB. | ||
| + # https://github.com/kubernetes/kubernetes/issues/19781 | ||
| + read_bufsize=2**21 |
There was a problem hiding this comment.
(Non blocking for this PR) Some of the patches like this seem to also apply to the syncio client.
There was a problem hiding this comment.
i'm not sure this applies to synchronous client, the reason to have this patch is due to the use of aiohttp as the comment mentioned.
-
# Watch events containing large resource objects can exceed -
# aiohttp's default read buffer size
| # Validate the run can be executed on Windows | ||
| if type(asyncio.get_event_loop()).__name__ == '_WindowsSelectorEventLoop': | ||
| raise ConfigException( | ||
| 'exec: _WindowsSelectorEventLoop does NOT support subprocesses, see README.md' |
There was a problem hiding this comment.
We also need to port some of the root README.md to this repo
|
Test results look good. /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: roycaihw, yliaog The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cc @tomplus |
|
I'm happy to see you are working on it. I'm ready to help but to be honest I'm worried about upgrading the generator. I'm not sure if you are aware of this issue: tomplus/kubernetes_asyncio#293 So I decided to stick to the old version of generator in my async lib. I applied patches if needed, I developed typing as a set of In addition, I'd suggest copying my entire lib into the kubernetes-client/python-asyncio. Could you consider this? I know, maintaining two libraries may be difficult, but I believe discrepancies between sync and async are inevitable and it wouldn't be any easier maintained in a single repo. Thanks! |
|
@tomplus it's great to have your help I was not aware of the issue tomplus/kubernetes_asyncio#293. I did a couple of quick tests using this repo(the openapi generator was upgraded to v6.6.0). ~/prj/client-python$ cat kubernetes_asyncio/.openapi-generator/COMMIT ~/prj/client-python$ time PYTHONPATH=. python -m kubernetes_asyncio.client.api_client the result is 0.5s, not the 12s. ~/prj/client-python$ time PYTHONPATH=. python examples_asyncio/list_pods.py real 0m1.883s the result is 1.8s. I think these results are acceptable, lazy imports may help futher improve it. what do you think? did i miss anything? |
the goal is to keep the synchronous and asyncio clients feature parity. Some features and bug fixes are available in synchronous client, but not in asyncio client, and vice versa. The major code lies in kubernetes/ and kubernetes_asyncio directories, i think keeping them in one repo is easier to ask contributors to fix bugs in both clients, and add features to both at the same time. another goal is to share the code between the two clients as much as possible, ideally, the only differences between the two are the inevitable ones, like await, async. I think one repo also makes it easier to share. For exmaple, the config/in_cluster_config is almost identical. For now, I kept them separate, it may be refactored in the future to share one file. $ diff kubernetes/config/incluster_config.py kubernetes_asyncio/config/incluster_config.py from kubernetes_asyncio.client import Configuration |
5 participants
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: