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

Part #1 of synchronous requests: Add channels and a mechanism for waiting #166

Merged
merged 1 commit into from
Jun 19, 2014

Conversation

brendandburns
Copy link
Contributor

Respective registries don't actually implement the channel behavior yet.

select {
case obj = <-out:
return obj, nil
case <-tick:
Copy link
Contributor

Choose a reason for hiding this comment

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

you could directly do: case <- time.After(timeout)

Copy link
Member

Choose a reason for hiding this comment

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

+1

@lavalamp
Copy link
Member

I had some other ideas on how to do this... let's chat tomorrow, maybe what I'm thinking won't work.

@proppy
Copy link
Contributor

proppy commented Jun 19, 2014

Something you can consider is keeping the synchronous operation and just wrapping the call in a goroutine.

outc := make(chan interface{})
errc := make(chan error)
go func() {
   obj, err := storage.Create(object)
   if err != nil {
      errc <- obj
      return
   }
   objc <- obj
}()
select {
  case obj := <-objc:
        // ...
  case err := <-errc:
        // ...
  case <- time.After(1 * time.Second):
        // ...
}

@brendandburns
Copy link
Contributor Author

Ok, I discussed this offline w/ @lavalamp and we came up with the following approach, I added a MakeAsync method that is used by the registries now, this means that individual storage's don't need deal in containers.

Take another look and let me know what you think.

@proppy
Copy link
Contributor

proppy commented Jun 19, 2014

I would have thought the MakeAsync logic would be around the ControllerRegistryStorage methods calls ot in their impl.


func MakeAsync(fn func() interface{}) <-chan interface{} {
channel := make(chan interface{}, 1)
go func() { channel <- fn() }()
Copy link
Member

Choose a reason for hiding this comment

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

inside this goroutine: defer util.HandleCrash()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@brendandburns
Copy link
Contributor Author

The problem with the go routine wrapping the whole thing is that you don't get immediate feedback for validation errors. In this approach, you get an immediate error if the request isn't accepted, and then you get a channel that you can optionally wait on for the result.

func MakeAsync(fn func() interface{}) <-chan interface{} {
channel := make(chan interface{}, 1)
go func() {
util.HandleCrash()
Copy link
Member

Choose a reason for hiding this comment

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

Has to be deferred, or it won't work. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done (oops!)

@brendandburns
Copy link
Contributor Author

Comments addressed, ptal.

Thanks
--brendan

lavalamp added a commit that referenced this pull request Jun 19, 2014
Part #1 of synchronous requests: Add channels and a mechanism for waiting
@lavalamp lavalamp merged commit 466be48 into kubernetes:master Jun 19, 2014
@lavalamp
Copy link
Member

btw, @brendandburns @brendanburns-- I'm working on some changes to cloudcfg, so you don't need to update that.

vishh pushed a commit to vishh/kubernetes that referenced this pull request Apr 6, 2016
xingzhou pushed a commit to xingzhou/kubernetes that referenced this pull request Dec 15, 2016
Fix typos in init container design.
iaguis pushed a commit to kinvolk/kubernetes that referenced this pull request Feb 6, 2018
wking pushed a commit to wking/kubernetes that referenced this pull request Jul 21, 2020
…-default

Add a sane default constructor for the Etcd and EtcdConfig structs
sjenning pushed a commit to sjenning/kubernetes that referenced this pull request Jul 27, 2020
brahmaroutu pushed a commit to brahmaroutu/kubernetes that referenced this pull request Sep 2, 2020
This reverts commit 278ece378a5054f16a07622b51ddaf82b328d6e6, reversing
changes made to 2df71ebbae66f39338aed4cd0bb82d2212ee33cc.
pjh pushed a commit to pjh/kubernetes that referenced this pull request Jan 31, 2022
* field capture

* update deployment specs to 1.1.4 for whereami

* fixed emoji bug

* updated readme example for path suffix

* make_response in app.py not needed

* json module in app.py not needed either

* missing paren in readme to address issue kubernetes#166
linxiulei pushed a commit to linxiulei/kubernetes that referenced this pull request Jan 18, 2024
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.

3 participants