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

Pagination for zone-lockdown #1023

Closed
tamas-jozsa opened this issue Aug 8, 2022 · 1 comment · Fixed by #1017
Closed

Pagination for zone-lockdown #1023

tamas-jozsa opened this issue Aug 8, 2022 · 1 comment · Fixed by #1017

Comments

@tamas-jozsa
Copy link

Current cloudflare-go version

latest

Description

ListZoneLockdowns should paginate through the results and return all ZoneLockdowns. It is especially important in cf-terraforming, that we can generate every zone lockdowns.

Also, taking the chance to bring methods to a common method signature, based on https://github.com/cloudflare/cloudflare-go/blob/master/docs/experimental.md

@jacobbednarz the documentation mentions Nested methods and services like client.Zones.List(). But I haven't found any case like that. Shall I update ZoneLockedowns similarly? client.ZoneLockdown.List()

Also, in the example you shared: https://github.com/cloudflare/cloudflare-go/blob/master/tunnel.go some method signatures differ from what the documentation suggests (sort of).

func (api *API) CreateTunnel(ctx context.Context, rc *ResourceContainer, params TunnelCreateParams) (Tunnel, error) 
func (api *API) UpdateTunnel(ctx context.Context, rc *ResourceContainer, params TunnelUpdateParams) (Tunnel, error)

Should we focus on a similar signature where we define different params type similar to (TunnelCreateParams, TunnelUpdateParams)

Or should we use a more standard example like:

func (api *API) CreateZoneLockdown(ctx context.Context, rc *ResourceContainer, ld ZoneLockdown) (ZoneLockdown, error)
func (api *API) UpdateZoneLockdown(ctx context.Context, rc *ResourceContainer, ld ZoneLockdown) (ZoneLockdown, error)

Use cases

cf-terraforming has to generate every zone lockdown.

Potential cloudflare-go usage

-

References

No response

@tamas-jozsa tamas-jozsa linked a pull request Aug 8, 2022 that will close this issue
9 tasks
@jacobbednarz
Copy link
Member

@jacobbednarz the documentation mentions Nested methods and services like client.Zones.List(). But I haven't found any case like that. Shall I update ZoneLockedowns similarly? client.ZoneLockdown.List()

https://github.com/cloudflare/cloudflare-go/blob/master/zones.go is an example of this pattern however, we shouldn't be taking that just yet. it's only available on the experimental client that we are still ironing out. you should use the tunnels examples and put the methods on the global client.

Should we focus on a similar signature where we define different params type similar to (TunnelCreateParams, TunnelUpdateParams)

yep, this. we want the methods to accept their own parameter structs as new resources don't always have the same parameters as an existing resource that needs to be updated in place.

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 a pull request may close this issue.

2 participants