Move chunk_size to argument and retry_on_failure = TRUE

[ldecicco-USGS]

After some testing, realized the "req_retry" function wasn't really doing much unless retry_on_failure = TRUE was set and httr2::req_perform_iterative needs to have on_error = "return" so that the retry actually gets triggered.

Additionally, moved the chunking argument to a top level argument.

[ldecicco-USGS]

Also a lot of argument rejiggering to follow tidyverse design principles:
https://design.tidyverse.org/dots-after-required.html

[jzemmels]

These updates look good. I have a few comments and questions, but nothing major. I ran check() locally and didn't see anything worrisome.

[jzemmels]

Unrelated but a question: do you know if hydrologic_unit_codes from the monitoring-location endpoint is a single or comma parameter? I was just testing out passing a list of HUC8s in drpy, and I got an answer, but the count was way small and I don't even know what was actually used.

[ldecicco-USGS]

When I last asked about that, huc's were not going to be comma parameters "any time soon" (so maybe someday?).

Remember HUCs are tricky. Our sites have a many combination of number of digits. So I think the issue was if you asked for c("012345", "012346"), you would only get those 2 HUCs, even if there were tons of sites with "01234567" and "01234678" for instance.

In dataRetrieval we send the HUC requests to a POST and that allows us to take advantage of the work the API developers put in to make HUCs special so that it really is going is as c("012345%", "012346%")

[ldecicco-USGS]

Oh wait! I forgot I set up the huc POST stuff specifically:
https://github.com/DOI-USGS/dataRetrieval/blob/develop/R/construct_api_requests.R#L498

[jzemmels]

I'm not sure what this means. Do you mean differentiate within the documentation file? Like a visual boundary between the API vs. dR-specific parameters?

[ldecicco-USGS]

Riffing off tidyverse's design principle:
https://design.tidyverse.org/dots-after-required.html

The idea is that the arguments after the ...'s are something rarely used and this requires that the users would specifically name them. Granted, in our functions, users mostly need to name the args anyway since there are so many (and no one should be defining ALL the arguments), but essentially the answer is yes - it's a good visual way to differentiate what are queries used in the API, and what we use outside the API (and therefore what users should either not mess with too often, and if they do they should understand what they're doing)

I also put "limit" in the dR-specific because it falls more into the category that users should rarely need to mess with it and it's confusing. They might think if they set the limit=10 they will only get 10 results, but it actually means they'll get 10 per page....so in that regard, since dR does automatic page walking.

[jzemmels]

Ahh, I understand. Thanks for clarifying

[jzemmels]

What are the dots used for here?

[ldecicco-USGS]

nothing, pulling them out now...

[jzemmels]

I accidentally ran this while at the API limit and got back the following output:

trying URL '[https://api.waterdata.usgs.gov/stac-files/ratings/USGS.01104475.exsa.rdb](vscode-file://vscode-app/c:/Users/jzemmels/AppData/Local/Programs/Positron/resources/app/out/vs/code/electron-browser/workbench/workbench.html#)'
Warning message:
In utils::download.file(url = url, destfile = full_file_path) :
  cannot open URL '[https://api.waterdata.usgs.gov/stac-files/ratings/USGS.01104475.exsa.rdb](vscode-file://vscode-app/c:/Users/jzemmels/AppData/Local/Programs/Positron/resources/app/out/vs/code/electron-browser/workbench/workbench.html#)': HTTP status was '429 Unknown Error'

Do we want the retry and print functionality to extend to this function as well?

[ldecicco-USGS]

good catch! I'll see if it acts the same way in the header as the ogc stuff

[ldecicco-USGS]

Well, the headers look the same for the request, but the download.file is outside httr2, so it won't be trivial to use something like the httr2::req_error(body = error_body) that we use with the APIs

[ldecicco-USGS]

Nevermind, got the download working with httr2 instead of the base download.file.

[jzemmels]

Is a list and those column names the expected output from users? Do you think users would prefer a row-bound, "tidy" data frame with an extra column indicating the site ID? I'm not saying I'd prefer the tidy format, just wondering what a user would expect.

[ldecicco-USGS]

The issue is that there is a lot of important information hidden in the "comment" attribute. So somehow we'd need to bundle that information in as well. On the other hand, the list means that each data frame acts the same as the traditional 1 site returned from readNWISrating.