Pagination for getMonitors

[alandotcom]

This adds pagination for getting monitors. It also returns the monitors uptime status based on the reported status from uptimerobot.

It also updates the CLI command to stream the responses using GetMonitorsChan.

[alandotcom]

Partially fixes #1

[bitfield]

Hey @lumberj! Awesome PR, thank you! I really appreciate it.

Getting the monitor status is a great improvement, and so is supporting pagination.

I'm interested in the channel approach to listing the monitors. What exactly is happening concurrently here? I mean, we're fetching the lists of monitors, and printing them out, but are we doing anything else at the same time? If not, why does it use a goroutine? If we want to print out monitors as we read them, rather than waiting until we've read all pages, would it be better to pass GetMonitors an io.Writer to write to?

[alandotcom]

That’s a good point. In the case where were printing things out, we could just use an io Writer and read from that.

In an another case where we might want to take the monitor results and do some blocking operation on them (e.g., send that uptime data to some other service), having a channel where we could read those results with N workers is useful.

On Sun, Jul 21, 2019 at 3:46 AM John Arundel notifications@github.com wrote:

Hey @lumberj https://github.com/lumberj! Awesome PR, thank you! I really appreciate it.

Getting the monitor status is a great improvement, and so is supporting pagination.

I'm interested in the channel approach to listing the monitors. What exactly is happening concurrently here? I mean, we're fetching the lists of monitors, and printing them out, but are we doing anything else at the same time? If not, why does it use a goroutine? If we want to print out monitors as we read them, rather than waiting until we've read all pages, would it be better to pass GetMonitors an io.Writer to write to?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitfield/uptimerobot/pull/11?email_source=notifications&email_token=AAN2NZCZTB2TLSYCBWDS6UTQAQ47RA5CNFSM4IFQTUV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2OAX3Q#issuecomment-513543150, or mute the thread https://github.com/notifications/unsubscribe-auth/AAN2NZEK623DT3ZDS66VKIDQAQ47RANCNFSM4IFQTUVQ .

[alandotcom]

Here's a contrived example of what I might want to do:

import (
    "fmt"
    "github.com/bitfield/uptimerobot/pkg"
    "log"
    "sync"
)

func doWork(monitors <-chan uptimerobot.Monitor, wg *sync.WaitGroup) {
    defer wg.Done()
        // work
}

func main() {
    client := uptimerobot.New("XXXX")
    monitorResponse := client.GetMonitorsChan()
    done := make(chan struct{})

    go func() {
        var wg sync.WaitGroup

        // Run 50 goroutines to read from <-Monitor and do some work
        for i := 0; i < 50; i++ {
            wg.Add(1)
            go doWork(monitorResponse.Monitors, &wg)
        }
        wg.Wait()
        close(done)
    }()

    // Block until we get an error from the monitorResponse.Error chan or work is done
    select {
    case err := <-monitorResponse.Error:
        log.Fatal(err)
    case <-done:
        return
    }
}

[bitfield]

I love goroutines and channels as much as anyone (more, probably), but I'm wary nowadays of making them part of my APIs. The sense I get is that concurrency should either be on the library side, or on the user side, but not straddling the API boundary, if you know what I mean. So we might make the monitor fetching concurrent, but return a single, complete list to the user in one go, or we might provide functions the user can call concurrently if they want to (for example NextPage() or something like that. The Google Cloud API does this.)

How would it be if we keep the new GetMonitors which implements, but hides, pagination, and hold GetMonitorsChan in abeyance for the time being while we think about how to fit it into the overall design scheme?

[alandotcom]

Thank you for the well thought out response. I’m still new to Go development.

I’ll update the PR with your suggestions.

I found this, https://inconshreveable.com/07-08-2014/principles-of-designing-go-apis-with-channels/, helpful to understand why so few APIs send/receive channels (and the many problems that might result when that does happen).

On Mon, Jul 22, 2019 at 4:05 AM John Arundel notifications@github.com wrote:

I love goroutines and channels as much as anyone (more, probably), but I'm wary nowadays of making them part of my APIs. The sense I get is that concurrency should either be on the library side, or on the user side, but not straddling the API boundary, if you know what I mean. So we might make the monitor fetching concurrent, but return a single, complete list to the user in one go, or we might provide functions the user can call concurrently if they want to (for example NextPage() or something like that. The Google Cloud API does this.)

How would it be if we keep the new GetMonitors which implements, but hides, pagination, and hold GetMonitorsChan in abeyance for the time being while we think about how to fit it into the overall design scheme?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitfield/uptimerobot/pull/11?email_source=notifications&email_token=AAN2NZF33DRFSJZK3ENYZJ3QAWH7FA5CNFSM4IFQTUV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2PSLTA#issuecomment-513746380, or mute the thread https://github.com/notifications/unsubscribe-auth/AAN2NZCSA4RMVBSGRDF5OK3QAWH7FANCNFSM4IFQTUVQ .

[alandotcom]

So we might make the monitor fetching concurrent, but return a single, complete list to the user in one go, or we might provide functions the user can call concurrently if they want to (for example NextPage() or something like that. The Google Cloud API does this.)

Do you prefer leaving the current function as is (return the first page only), or, paginate and collect all the results then return the single set of results?

Thank you for pointing me to the Google Cloud API. They do have a section for implementing iterators,

https://github.com/googleapis/google-cloud-go/wiki/Iterator-Guidelines

I also found this a good read on three possible patterns, https://blog.kowalczyk.info/article/1Bkr/3-ways-to-iterate-in-go.html

They also recommend the iterator approach, as it provides the best API, and is more common.