search

ropensci / weathercan

R package for downloading weather data from Environment and Climate Change Canada
https://docs.ropensci.org/weathercan
GNU General Public License v3.0
102 stars 29 forks source link

check status on GET requests #120

Closed KevCaz closed 3 years ago

KevCaz commented 3 years ago

Following-up on #119,

I went a bit further than just adding a function check_status(), I introduce get_check() that handles the GET() requests and check status, which includes (1) a check of whether data are available and (2) the use of httr::stop_for_status() as you are already using.

get_check() is used in the four main access points. I then checked the package, everything is fine, I just had to adjust one test, so it uses the new error message thrown. BTW, I think it would be worth turning the message below

https://github.com/ropensci/weathercan/blob/2bcad0938508d2760a96f354dd4066bb8c429b65/R/stations.R#L156-L159

into a message used by argument task in get_check(). Let me know what do you think about all this.

steffilazerte commented 3 years ago

So excited to look at this! I'll need a day or so to do so, but I can't wait :grin:

KevCaz commented 3 years ago

Hi @steffilazerte, Thanks! I'll have a look at all this later today or tomorrow, I was also thinking that I could add a simple test with the "server is down" URL, what do you think?

steffilazerte commented 3 years ago

I think that'd be fantastic! Thanks!

KevCaz commented 3 years ago

Hi @steffilazerte,

I'm basically done, let me know if I need to do something else, again thanks for all your work on this, I know a few people using weathercan for their research project and they all very grateful for its existence, which I guess means that they all are grateful for your work on this project!

steffilazerte commented 3 years ago

Wonderful, thanks @KevCaz! That's great to hear that and also lovely to have contributions! I'll merge this in and finish prepping for the CRAN release. Take care!

KevCaz commented 3 years ago

@steffilazerte, sorry, I realize I forgot to add a new item in the NEWS.md... something like the one you proposed sounds good enough to me "better error checking has been for GET requests (see #120)".

steffilazerte commented 3 years ago

Not a problem! I've added it in, and am now patting myself on the back for creating check_eccc() (returns TRUE/FALSE depending if it can ping the website) and wrapping all my examples in if(check_eccc()) HA! hopefully no more failures.

KevCaz commented 3 years ago

Good luck!!