Skip to content

Add Google Cloud Storage support#59

Merged
jimhester merged 3 commits intor-lib:masterfrom
MarkEdmondson1234:master
Mar 19, 2018
Merged

Add Google Cloud Storage support#59
jimhester merged 3 commits intor-lib:masterfrom
MarkEdmondson1234:master

Conversation

@MarkEdmondson1234
Copy link
Contributor

@MarkEdmondson1234 MarkEdmondson1234 commented Nov 27, 2017

I tried to keep to the same format as the S3 integration.

I made a bucket on GCS called memoise-tests that I can give you access to as well for the tests to work as is (after you add your own authentication file) - just let me know your Google email to add it to. The tests all work for me locally, for them to work for you too it would need these authentication environment arguments set:

  • GCS_AUTH_FILE - pointing at the service account JSON
  • GCS_DEFAULT_BUCKET - pointing at the GCS bucket you have pre created, either on the web UI or via googleCloudStorageR::gcs_create_bucket()
  • GAR_CLIENT_JSON to set your own client_id and secret via googleAuthR::gar_set_client() in the tests. It also assumes you have activated Cloud platform scope (on by default on GCPs) This is also downloaded from a Google Cloud project, another JSON file. See the help. Or you can set it directly via options(googleAuthR.client_id) and options(googleAuthR.client_secret) but I prefer the former.

The last one is only due to running non-interactivly in the tests, usually it will get set when you load library(googleCloudStorageR) but I didn't think that was a good idea in the tests.

I did deviate a bit by not creating a new bucket each time like that is done in the S3 function, I don't think thats necessary and its a lot smoother with a pre-made one. I also don't create a new bucket if its not there, I'd prefer more control when that is done, but you may disagree, I'll defer to whatever you think best.

Thanks for the package, its really useful, I use it for caching API calls in other packages.

@codecov-io
Copy link

codecov-io commented Nov 27, 2017

Codecov Report

Merging #59 into master will decrease coverage by 42.13%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #59       +/-   ##
===========================================
- Coverage     100%   57.86%   -42.14%     
===========================================
  Files           4        5        +1     
  Lines         151      197       +46     
===========================================
- Hits          151      114       -37     
- Misses          0       83       +83
Impacted Files Coverage Δ
R/cache_gcs.R 0% <0%> (ø)
R/cache_s3.R 0% <0%> (-100%) ⬇️
R/memoise.R 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d63ae9c...a33b512. Read the comment docs.

@jimhester
Copy link
Member

Thanks! This looks fine, if you can just add a note to NEWS.md with the change and mentioning your GitHub username and this issue I can merge.

@MarkEdmondson1234
Copy link
Contributor Author

Thanks @jimhester - added those bits now.

@jimhester
Copy link
Member

Thanks!

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 this pull request may close these issues.

3 participants