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

Allow for custom timeout in watch request #322

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

pshchelo
Copy link
Contributor

@pshchelo pshchelo commented Mar 6, 2020

What do these changes do?

allow to set a custom timeout when making the GET ..?watch=true request to Kubernetes API if needed.

Description

Currently the timeout is always None, which we find to be a reason for issues like #204 .
With this patch controller authors may override the timeout passed by kopf to the underlying aiohttp session request by doing something like this in their controller code:

import kopf

kopf.config.WatchersConfig.session_timeout = 10.0

@kopf.on(..)
def handler(*):
    ....

The default behavior is not changed.

Issues/PRs

Issues: #204

Type of changes

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • The code addresses only the mentioned problem, and this problem only
  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt

@pshchelo pshchelo requested a review from nolar as a code owner March 6, 2020 15:57
@zincr
Copy link

zincr bot commented Mar 6, 2020

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@pshchelo
Copy link
Contributor Author

interestingly enough, coveralls job fails and reports as not covered lines I did not change/add...

@nolar
Copy link
Contributor

nolar commented Mar 10, 2020

Coveralls decrease is all fine. It fluctuates a little bit here & there.

Regarding the feature: Can you please refactor it to make the timeout as float instead of aiohttp.ClientTimeout?

Usage of aiohttp is a hidden detail of the current implementation and can change in the future. The session/client timeouts will probably stay, just differently implemented inside of kopf.clients.*.

UPD: Actually, Optional[float] = None, not just float.

@pshchelo
Copy link
Contributor Author

Ack, sounds reasonable, will do

this will allow to set a custom value for (total) session timeout
when making the ?watch=true request to Kubernetes API if needed.

related issue 204
@pshchelo pshchelo force-pushed the custom-watch-timeout branch from 4c2cfdf to e259317 Compare March 16, 2020 09:37
@nolar nolar requested a review from haikoschol March 17, 2020 07:47
@nolar nolar merged commit a2c2ffb into zalando-incubator:master Mar 18, 2020
@pshchelo pshchelo deleted the custom-watch-timeout branch March 19, 2020 11:50
@nolar nolar added this to the 0.27 milestone May 11, 2020
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.

3 participants