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

Attempting to load tidyhydat with no internet causes an error #149

Closed
joethorley opened this issue Feb 4, 2021 · 4 comments · Fixed by #150
Closed

Attempting to load tidyhydat with no internet causes an error #149

joethorley opened this issue Feb 4, 2021 · 4 comments · Fixed by #150

Comments

@joethorley
Copy link

Attempting to load tidyhydat with no internet causes an error - is it possible to replace the error with a warning so that scripts that use tidyhydat can be run without an internet connection?

> library(tidyhydat)
● Checking for a new version of HYDAT...

Error: package or namespace load failed for ‘tidyhydat’:
 .onAttach failed in attachNamespace() for 'tidyhydat', details:
  call: NULL
  error: No access to internet
Backtrace:
    █
 1. └─base::library(tidyhydat)
 2.   └─base::tryCatch(...)
 3.     └─base:::tryCatchList(expr, classes, parentenv, handlers)
 4.       └─base:::tryCatchOne(expr, names, parentenv, handlers[[1L]])
 5.         └─value[[3L]](cond)
> 
@boshek
Copy link
Collaborator

boshek commented Feb 4, 2021

👋 @joethorley

Yep that's a good call. Seems like I really just need to check sooner if the user has internet then I don't need the hard stop. Any thoughts on how to write a test that mimics no internet?

@joethorley
Copy link
Author

@boshek - that's great. It's never anything I've done before.

This may be of interest

https://community.rstudio.com/t/unit-testing-for-non-working-internet-connection/31894

@boshek
Copy link
Collaborator

boshek commented Feb 4, 2021

@joethorley are you able to check to see if the changes on the no-internet branch work for your purposes?

remotes::install_github("ropensci/tidyhydat")

Still trying to assess whether I actually need a test for this.

@joethorley
Copy link
Author

That works for me - it doesn't issue a warning on load but in my opinion this is fine (even desirable) and then errors when attempting to download which is desirable with an informative error.

I'm not sure that a test is required.

> library(tidyhydat)
● Checking for a new version of HYDAT...

x Your version of HYDAT is out of date. Use download_hydat() to get the new version.

# switch off wifi

> library(tidyhydat)
> tidyhydat::download_hydat()
Downloading HYDAT will take ~10 minutes.
This will remove any older versions of HYDAT
Is that okay?
────────────────────────────────────────────────────────────────────────────────────────────

1: Yes
2: No

Selection: 1Downloading HYDAT.sqlite3 to ~/Library/Application Support/tidyhydat
Error in curl::curl_fetch_memory(url, handle = handle) : 
  Could not resolve host: collaboration.cmc.ec.gc.ca
Backtrace:1. └─tidyhydat::download_hydat()
 2.   └─httr::GET(base_url)
 3.     └─httr:::request_perform(req, hu$handle$handle)
 4.       ├─[ httr:::request_fetch(...) ]
 5.       └─httr:::request_fetch.write_memory(req$output, req$url, handle)
 6.         └─curl::curl_fetch_memory(url, handle = handle)

boshek added a commit that referenced this issue Feb 4, 2021
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

Successfully merging a pull request may close this issue.

2 participants