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

SDK inconsistency in zone vs record APIs #135

Closed
ideahitme opened this issue Jun 13, 2017 · 4 comments
Closed

SDK inconsistency in zone vs record APIs #135

ideahitme opened this issue Jun 13, 2017 · 4 comments

Comments

@ideahitme
Copy link

Description:

Zones sdk code does not retrieve all zones, only the first page.
DNS records sdk code does retrieve all records

Details:
This is a snippet from zone.go ListZones(z ...string) method.

		// TODO: Paginate here. We only grab the first page of results.
		// Could do this concurrently after the first request by creating a
		// sync.WaitGroup or just a channel + workers.
		res, err = api.makeRequest("GET", "/zones", nil)
		if err != nil {
			return []Zone{}, errors.Wrap(err, errMakeRequestError)
		}
		err = json.Unmarshal(res, &r)
		if err != nil {
			return []Zone{}, errors.Wrap(err, errUnmarshalError)
		}
		zones = r.Result

As noted in TODOs it does not implement pagination, moreover the struct where JSON is unmarshaled does not define (as opposed to response from dns records API):

// ResultInfo contains metadata about the Response.
type ResultInfo struct {
	Page       int `json:"page"`
	PerPage    int `json:"per_page"`
	TotalPages int `json:"total_pages"`
	Count      int `json:"count"`
	Total      int `json:"total_count"`
}
  1. I would assume ResultInfo is still part of the http response from "https://api.cloudflare.com/client/v4/zones and can be retrieved ?
  2. Are u planning to have this fixed, I could create a PR if it is not yet planned ?
@Tenzer
Copy link
Contributor

Tenzer commented Jun 13, 2017

Related issues: #6 and #4.

@ideahitme
Copy link
Author

As per official documentation https://api.cloudflare.com/#zone-list-zones it has

type ResultInfo struct {
	Page       int `json:"page"`
	PerPage    int `json:"per_page"`
	TotalPages int `json:"total_pages"`
	Count      int `json:"count"`
	Total      int `json:"total_count"`
}

as json, so in theory it should be pretty simple to implement

@ideahitme
Copy link
Author

@Tenzer I see, feel free to close this if you think it is necessary

@stale
Copy link

stale bot commented May 13, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label May 13, 2019
@stale stale bot closed this as completed Jun 12, 2019
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

No branches or pull requests

2 participants