Can we switch to using chan/signal based waits

[james-nesbitt]

Is there a way that we can switch from timeoutes to channel based controls for http operations, and for the wait operations?

The concept of "waiting for a process to finish" has many pitfalls. Using things like bool chans to mark when a process is finished gives lib consumers many more options on coding using the api.

Using something like https://godoc.org/golang.org/x/net/context is pretty standard. It provides a wrappable context that allows for timeouts, but also provides alternate cancellation approaches, and natively handles "cancel functions". This functionality is easily integrated into the net/http libraries, and is commonly used by big projects (like docker/libcompose.)

Reasoning:

1. using timeouts is limiting, as it provides no alternative break methods to be combined
2. Go suggest chans and subroutines, over waits

Ideas:

1. Make all http requests use https://godoc.org/golang.org/x/net/context for all http related actions , instead of just setting a timeout. This can be done internally first, and then context could be assignable in the service or client, and perhaps also made a part of any service methods (that would be a pretty big API change.)

2. Make all of the Wait methods use https://godoc.org/golang.org/x/net/context instead of timeouts. This means that code that uses the library can use any form of context shutdown, and consuming code can then track chans instead of waiting for a function to call.

Goal:

Convert the WaitForXXX methods (and all http requests) to more of a "fire-and-forget" forked approach, as opposed to a functional in-line "wait-for-finish" approach.

Required effort:

1. The client would need a way to receive a context (either across it's entire scope, or per request)

2. The client.PerformGetRequest() method would need to attach the context to the http Request using https://golang.org/pkg/net/http/#Request.WithContext

3. The request.WaitForXXX structs should accept a https://godoc.org/golang.org/x/net/context instead of a timeout.

4. The service.WaitForXXX functions should check the context->Done() chan for exiting, instead of checking an overall timeout (the duration based sleep should also use a small timeout context instead of a time.Sleep())

What do you think? I could give you a PR if you can give me some feedback on the ideas.

[Jalle19]

I must say I haven't used Go that much so I only have a vague understanding of channels and goroutines. Feel free to make a PR, if possible using an approach that changes the public API as little as possible.

[james-nesbitt]

Understandable request. I will start by duplicating the WaitX methods to new chan/context based approaches to demonstrate the change. Then I can show you some of the advantages.

I will leave the http request mechanism alone for now.

In case it isn't clear, I will create a fork, and start a PR from the fork.

[Jalle19]

Thanks, please do that!

[james-nesbitt]

I did manage to take a first run at this and failed miserably. Will try again soon.

I am looking at the following goals for the WaitXXX methods:

1. be able to show some progress (upcloud.Server.Progress)
2. be able to cancel the wait from outside of the wait (on input event)
3. be able to run multiple WaitXXXs in parrallel (without writing big wrappers)

[PopoSensei]

Hello from 2020. There are new changes to this API/SDK, go update among others, and I am reviewing all the old issues. Thinking of closing this issue, since no PR has been issued and this hasn't bubbled as a request, though it might in the future. Will give you time, before closing, to post a plea for future feature request and we will consider upgrading this feature request again. :)

[kangasta]

Hello! We are now using contexts for both requests (see v4.7.0 and v5.0.0) and WaitFor* methods (see v7.0.0), so I'll close this as completed 🚢