Conversation
|
I started off by adapting the leaderelection module from |
33c2ad8 to
c7de87d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #347 +/- ##
==========================================
+ Coverage 26.91% 27.05% +0.13%
==========================================
Files 799 805 +6
Lines 98316 98706 +390
==========================================
+ Hits 26463 26704 +241
- Misses 71853 72002 +149 ☔ View full report in Codecov by Sentry. |
No worries, I usually adapt with modifications. Thanks! |
tomplus
left a comment
There was a problem hiding this comment.
Excellent job, as always.
Thank you, Jacob.
c7de87d to
112c3d4
Compare
Add leaderelection module, based off of the leaderelection module in kubernetes-client/python. The module has been altered slightly to support asyncio. Fixes tomplus#297
112c3d4 to
cce9f8b
Compare
Add leaselock, derived from [1]. [1]: https://github.com/kubernetes-client/python/pull/2314/files
|
I agree, some of them should be on debug level... |
Instead of passing Callable[...,Coroutine] functions as callbacks, pass Coroutines directly. This allows the caller to supply arbitrary context associated with the Coroutine, such as the ApiClient used to establish the leader election.
* Remove misplaced basicConfig * Use %-style string interpolation
I switched to a mix of debug for verbose debug conditions, and error/exception for messages that actually indicate problems. Let me know if you agree with the selections. I also made an unrelated change in ef01ee6, switching the callback functions from async with api_client.ApiClient() as apic:
# Create config
leader_election_config = electionconfig.Config(
# A legacy ConfigMapLock is also available
LeaseLock(lock_name, lock_namespace, candidate_id, apic),
lease_duration=17,
renew_deadline=15,
retry_period=5,
onstarted_leading=example_start_func(api_client=apic),
onstopped_leading=example_end_func(),
)
# Enter leader election
await leaderelection.LeaderElection(leader_election_config).run()This seemed to be much more pleasant than other options for including arbitrary context, such as defining otherwise-unnecessary wrapper functions, using contextvars, etc. Let me know what you think. |
|
I see two potential problems with passing coroutines:
what about supporting two versions? we can check if it is a function or a coroutine and act accordingly. |
For the function version, are you suggesting supporting non-async functions, or supporting async functions (that return coroutines)? |
async functions and coroutines: |
* Add leaderelection module Add leaderelection module, based off of the leaderelection module in kubernetes-client/python. The module has been altered slightly to support asyncio. Fixes #297 * Add leaselock Add leaselock, derived from [1]. [1]: https://github.com/kubernetes-client/python/pull/2314/files * Switch callbacks to coroutines Instead of passing Callable[...,Coroutine] functions as callbacks, pass Coroutines directly. This allows the caller to supply arbitrary context associated with the Coroutine, such as the ApiClient used to establish the leader election. * Logging fixes * Remove misplaced basicConfig * Use %-style string interpolation * Revise log levels * Accept both coroutines and coroutine functions
Add leaderelection module, based off of the leaderelection module in kubernetes-client/python. The module has been altered slightly to support asyncio.
Fixes #297