search

bcgov / bcdata

An R package for searching & retrieving data from the B.C. Data Catalogue
https://bcgov.github.io/bcdata
Apache License 2.0
81 stars 12 forks source link

Reconcile chunk_limit and single_download_limit #332

Closed ateucher closed 9 months ago

ateucher commented 9 months ago

330 surfaced the change to the single download limit on the WFS server to 500 records (down from 10,000). This exposed the potential incompatibility between bcdcd.single_download_limit and bcdc.chunk_limit options, where it was possible for the latter to be larger than the former. Since bcdcd.single_download_limit determined whether or not we needed pagination for a request, and bcdc.chunk_limit is used to determined the page size (number of records requested in a page), we were getting into the situation where we were attempting to retrieve more records in a page than is allowed by the server.

This PR sets the default chunk limit to the bcdc.single_download_limit by default, and also checks more comprehensively that the two values are compatible.

This address part of #330, but unfortunately does not fix all of it.

ateucher commented 9 months ago

All passing now!

ateucher commented 9 months ago

Thanks both! I guess the remaining question is - should I remove one of the options (likely bcdata.single_download_limit, as that should always be retrieved from the server)? I think it would remove a lot of the gymnastics we're doing now, and I can't see the case where someone would want to set both manually.

stephhazlitt commented 9 months ago

Is there ever a use case where a user would want to set a lower bcdata.single_download_limit? If no, then it makes sense to remove the option.

ateucher commented 9 months ago

Not really - as it can be done with the chunk limit.

boshek commented 9 months ago

I think that if we do remove it, we should probably properly deprecated. I'd advocate just to leave it for now rather than risk breaking anyone's code.

ateucher commented 9 months ago

Summary of new changes @boshek @stephhazlitt: