add .onLoad() and .onAttach() functions which call use_credentials() … #184
Conversation
…if necessary, closes #182
|
I can't reproduce the test failure in Travis. I don't think I introduced any issues. I checked it locally with the same command as used in Travis ( In the travis logs I see two issues that could have caused the failure, both in this snippet: The first is that it looks like the name of the package is mangled, or those are just non-ascii quotes around it that are being rendered funny. Any chance this can be fixed, so that Travis will be happy and the PR can be considered for merging? Thanks. |
R/utils.R
Outdated
| # If credentials are in environment variables, use those. | ||
| creds <- aws.signature::locate_credentials() | ||
| if (!all(is.null(creds$key), is.null(creds$secret))) { | ||
| return |
There was a problem hiding this comment.
I think this should be something other than just the return function. Do you mean return(NULL) or return(invisible(NULL) perhaps?
R/utils.R
Outdated
| } | ||
| } | ||
|
|
||
| # Handle when package is attached (invoked with pkg::function()) not loaded |
There was a problem hiding this comment.
You don't need this function. .onLoad() is called when a package namespace is loaded, which occurs either at library("aws.s3") or aws.s3::{anything}(). .onAttach() is called only when library() or require() is used and is discouraged: https://cran.r-project.org/doc/manuals/r-devel/R-exts.html#index-_002eonAttach
.onLoad() is not meant to return anything, it's called for its side effect of finding the credentials. But to be explicit, it now returns invisible(NULL) if the credentials cannot be found. Also, the .onAttach() function has been removed.
Codecov Report
@@ Coverage Diff @@
## master #184 +/- ##
==========================================
+ Coverage 20.72% 20.74% +0.02%
==========================================
Files 32 32
Lines 1110 1128 +18
==========================================
+ Hits 230 234 +4
- Misses 880 894 +14
Continue to review full report at Codecov.
|
|
OK, the changes have been made. It looks like the earlier build error was my fault, I fixed it by checking for the existence of |
|
Thanks. I've introduced this functionality into aws.signature so that it is consistent across cloudyr packages. cloudyr/aws.signature@7c44c5e |
…if necessary, closes #182
Added a hook at package startup/attach time which (if creds have not been supplied via environment variables) runs
aws.signature::use_credentials()so that users withaws profiles (in
~/.aws) do not have to manually type that command before they can use S3.