socketry / cloudflare

An asynchronous Ruby wrapper for the CloudFlare V4 API.
MIT License
139 stars 88 forks source link

Pagination does not seem to work #64

Closed patcable closed 3 years ago

patcable commented 4 years ago

For the following code:

require 'cloudflare'
dns_records = nil
Cloudflare.connect(key: api_key, email: email) do |connection|
  zone = connection.zones.find_by_name('mydomain.info')
  dns_records = zone.dns_records.get
end
dns_records.results.length

dns_records.results.length outputs 20. I definitely have more than 20 records.

Does Paginate.each only paginate zones and not dns records inside of zones? I'd be OK doing zone.dns_records.get.find_by_nameto get a result as well, but that only returns a record if it's in the first 20 returned results which is also a problem.

patcable commented 4 years ago

Oh, I also just noticed - https://github.com/socketry/cloudflare/blob/master/lib/cloudflare/dns.rb#L83 - why call .first? Cloudflare's API returns each address as a separate record, so that would only return the first record, but a name with multiple IP addresses will have a unique record for each IP address

martinblaha commented 3 years ago

Same issue here: how do I get more than 20 items back as response?

...
zone = connection.zones.find_by_name(DOMAIN)
dns_records = zone.dns_records.get # this returns only 20 items
---

Thank you.

ioquatix commented 3 years ago

Does zone.dns_records.each give all the records?

martinblaha commented 3 years ago

Does zone.dns_records.each give all the records?

No. Still only 20:

dns_records.body[:result].count
=> 20

I'm wondering if there is a way how to pass "per_page" to my query dns_records = zone.dns_records.get # < param possible?. According to https://api.cloudflare.com/#getting-started-requests (see Pagination), we can pass per_page param like this

curl -X GET "https://api.cloudflare.com/client/v4/zones/cd7d068de3012345da9420df9514dad0/dns_records?page=3&per_page=20&order=type&direction=asc" \
     -H "Content-Type:application/json" \
     -H "X-Auth-Key:1234567893feefc5f0q5000bfo0c38d90bbeb" \
     -H "X-Auth-Email:example@example.com"
rdubya commented 3 years ago

iirc Cloudflare's api requires pagination unless you have a small enough number that you can fit it in a page. I think that calling dns_records.each.entries will give you an array of all of them, but I'm not somewhere that I can test that right now. You can see how each is implemented here: https://github.com/socketry/cloudflare/blob/master/lib/cloudflare/paginate.rb#L25

martinblaha commented 3 years ago

Does zone.dns_records.each give all the records?

It does, you are right - my fault. This returns an enumerator with all items which I can iterate on:

zone.dns_records.each do |rec|
  puts(rec)
end

Thank you. This gets me going again :-)

martinblaha commented 3 years ago

iirc Cloudflare's api requires pagination unless you have a small enough number that you can fit it in a page. I think that calling dns_records.each.entries will give you an array of all of them, but I'm not somewhere that I can test that right now. You can see how each is implemented here: https://github.com/socketry/cloudflare/blob/master/lib/cloudflare/paginate.rb#L25

zone.dns_records.entries also works and returns all entries at once which again I can iterate on. Thank you.

My only problem left is that the call zone.dns_records.entries takes ~90 sec for 85 entries. Are there any strategies how to speed this up?

ioquatix commented 3 years ago

Well, the performance will depend on how fast Cloudflare is. Because you have to fetch one page at a time.

ioquatix commented 3 years ago

Maybe you can set per_page to 100 so it gets them in a single request?