exoscale / egoscale

exoscale golang bindings
https://pkg.go.dev/github.com/exoscale/egoscale/v3
Apache License 2.0
31 stars 14 forks source link

Add support for external-sources #525

Closed pyr closed 3 years ago

shortcut-integration[bot] commented 3 years ago

This pull request has been linked to Clubhouse Story #32856: security groups: allow adding external addresses.

falzm commented 3 years ago

LGTM

(can't we get rid of the oapi.NewPoller() duplication ?)

What duplication? Those are 2 different execution scopes (2 different methods).

pyr commented 3 years ago

What duplication? Those are 2 different execution scopes (2 different methods).

You mean reducing this to something more succinct since it's code that lives in many of these methods?

    _, err = oapi.NewPoller().
        WithTimeout(c.timeout).
        WithInterval(c.pollInterval).
        Poll(ctx, c.OperationPoller(zone, *resp.JSON200.Id))
    if err != nil {
        return err
    }

If so, I don't think it would belong in this PR since it is the way the whole code base is structured, deviating here would introduce an exception for these two calls.

pyr commented 3 years ago

Thanks @falzm for helping with the tests!