Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controller<T> #184

Closed
wants to merge 8 commits into from
Closed

Controller<T> #184

wants to merge 8 commits into from

Conversation

clux
Copy link
Member

@clux clux commented Mar 9, 2020

Sketch of Controller. Heavy WIP for #148

Some thoughts:

  • using a pull based model like in Informer because it feels nicer than callbacks, and is consistent
  • probably will use some kind of signalling to ensure we can debounce events and notify the owner of what does the parallel poll when it should release an item into the stream

@clux clux changed the title Controller<T> Controller<T> - for #148 Mar 10, 2020
@clux clux changed the title Controller<T> - for #148 Controller<T> Mar 10, 2020
@clux clux force-pushed the controller-start branch 2 times, most recently from 51239fd to cea8d9a Compare March 13, 2020 19:37
// Prepare a sender so we can stack things back on the channel
let txa = tx.clone();
// Event loop that triggers the reconcile fn
tokio::spawn(async move {
Copy link
Member

@nightkr nightkr Mar 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense while prototyping.. but we should try to avoid tokio::spawn before releasing, because it means that kube-rs has to take complete control of the error handling. Should be possible to replace most uses with some variant of try_join_all/try_join!.

impl<K: 'static, F: 'static> Controller<K, F>
where
K: Clone + DeserializeOwned + Meta + Send + Sync,
F: Fn(ReconcileEvent) -> Result<ReconcileStatus> + Send + Sync,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is pretty limiting that F can't return an impl Future.

@clux clux force-pushed the controller-start branch from dc8bfd6 to 242b1b0 Compare March 28, 2020 14:03
@clux
Copy link
Member Author

clux commented Jul 17, 2020

Replaced by #258

@clux clux closed this Jul 17, 2020
@clux clux deleted the controller-start branch July 17, 2020 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants