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

Provide convenience methods for paging over collections #292

Open
timoreimann opened this issue Jan 13, 2020 · 0 comments
Open

Provide convenience methods for paging over collections #292

timoreimann opened this issue Jan 13, 2020 · 0 comments
Labels
do-api Depends on changes to the DigitalOcean API enhancement

Comments

@timoreimann
Copy link
Contributor

A number of API responses return collections (i.e., a slice of objects as opposed to just a single object) that callers need to page through in order to visit exhaustively. Examples are droplets, volumes, and Kubernetes clusters.

While the implementation isn't necessarily complex, the boilerplate adds up quickly when multiple collection object types are involved or iteration needs to happen repeatedly in client code. What's usually happening in my code then is that abstractions emerge that allow me to reuse paging. However, that approach doesn't work across repositories.
Consequently, I'd like to kick off a discussion on the possibility of providing convenience methods for iterating over collections in godo; essentially, my ask is to move the kind of abstraction I talked about above into godo.

Assuming for a second that this is an idea we can get onboard with in general, the interesting question is how it could be implemented, which in turn depends on what we imagine the signature to look like. One challenge here may be that different collections return different types, and Go's lack of generics does not allow us to (easily) provide a single implementation.

My basic thinking is that we offer a method which accepts a function to handle a given (paged over) object, with possibilities to indicate if paging should stop prematurely as well as error propagation. The scaffolding of increasing the page variable and handling paging-related errors should be done by the outer implementation, not the passed in custom function from the user.

With that in mind, here's my initial proposal (using Volumes as an example):

We'd provide

type VolumeHandler func(context.Context, Volume, *Response) (done bool, err error)

and extend the StorageService interface by a new method:

PageVolumes(context.Context, VolumeHandler, *ListVolumeParams) error

The implementation of PageVolumes could then look something like this:

func (svc *StorageServiceOp) PageVolumes(ctx context.Context, handler VolumeHandler, params *ListVolumeParams) error {
    for {
        volumes, resp, err := svc.Client.ListVolumes(ctx, params)
        if err != nil {
            return err
        }

        // handle each volume by the caller's handler function
        for _, vol := range volumes {
            done, err := handler(ctx, vol, resp)
            if err != nil {
                return err
            }
            if done {
                return nil
            }
        }

        // if we are at the last page, break out the for loop
        if resp.Links == nil || resp.Links.IsLastPage() {
            break
        }

        page, err := resp.Links.CurrentPage()
        if err != nil {
            return err
        }

        // set the page we want for the next request
        params.ListOptions.Page = page + 1
    }

    return nil
}

I realize the proposal may be far from (our) ideal; and there are likely detailed design aspects we'd have to discuss (e.g., whether we should pass in the *Response into the handler function or not). More importantly though, there's the question of whether this is the right approach on a higher level: Should this be a new method on each interface? Should we consider code generation to avoid repeating lots of code? Or is there perhaps a chance to centralize the implementation by using interface{}s, at the price of less type safety?

My primary goal with this issue is to start a discussion, so I'm very open to exploring any directions you may feel are more appropriate.

Thanks!

@ChiefMateStarbuck ChiefMateStarbuck added enhancement do-api Depends on changes to the DigitalOcean API labels Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-api Depends on changes to the DigitalOcean API enhancement
Projects
None yet
Development

No branches or pull requests

2 participants