Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Convert client wrappers to coroutines, use thread pools for actual calls #217

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Oct 28, 2019

Issue : #142

Description

The client wrappers are used to isolate the framework's reactor from the specific HTTPS API calls needed to talk to Kubernetes, and from the ways of doing so. They provide a limited set of operations needed only, and no more than that (no general-purpose DSL for resource manipulation).

In this PR, all remaining synchronous client wrappers are converted to async coroutines, while they remain Pykube-based internally (pykube-ng is synchronous, as requests are synchronous).

This is needed both for the switch to aiohttp (e.g. #176), and for the re-authentication activities (few issues).

The synchronous calls are moved few levels down the stack, and put into the thread-pool executors.

Previously, these calls were sync inside of the async event loop, so they were blocking. It was not a big problem, as most of them are executed only once or twice per run in the very beginning; the frequent resource-watching & resource-patching & event-posting calls were already executed in the thread pools. But it now becomes a problem since all API calling methods cannot be so diverse anymore, and have to be uniform (specifically, async).

Types of Changes

  • Refactor/improvements

@nolar nolar added the refactoring Code cleanup without new features added label Oct 28, 2019
@nolar nolar requested a review from samurang87 as a code owner October 28, 2019 19:29
@zincr
Copy link

zincr bot commented Oct 28, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Oct 28, 2019

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@nolar
Copy link
Contributor Author

nolar commented Nov 6, 2019

Zincr-bot, ping.

@nolar nolar removed the request for review from samurang87 November 6, 2019 17:30
@perploug perploug self-requested a review November 7, 2019 08:59
@nolar nolar merged commit 6654532 into zalando-incubator:master Nov 7, 2019
@nolar nolar deleted the async-clients branch November 7, 2019 09:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
refactoring Code cleanup without new features added
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants