Add new `board_gcs()` for Google Cloud Storage #695

juliasilge · 2022-12-20T01:02:30Z

Closes #572

juliasilge · 2022-12-20T01:27:25Z

As I am working through this, I have a few questions that I hope you can help me with @MarkEdmondson1234.

The first is the logging/messaging that googleCloudStorageR does:

library(pins)
googleCloudStorageR::gcs_auth("~/google-pins.json")
b <- board_gcs("pins-dev")
b %>% pin_write(1:10, "small-numbers", type = "rds")
#> Creating new version '20221220T011855Z-4d1eb'
#> ℹ 2022-12-19 18:18:55 > File size detected as  189 bytes
#> 
#> ℹ 2022-12-19 18:18:55 > File size detected as  61 bytes
#> 
#> Writing to pin 'small-numbers'
b %>% pin_write(100:110, "big-numbers", type = "json")
#> Creating new version '20221220T011856Z-b89e8'
#> ℹ 2022-12-19 18:18:56 > File size detected as  187 bytes
#> 
#> ℹ 2022-12-19 18:18:56 > File size detected as  46 bytes
#> 
#> Writing to pin 'big-numbers'
b %>% pin_write(1:10, "small-numbers", type = "json")
#> Creating new version '20221220T011856Z-c3943'
#> ℹ 2022-12-19 18:18:57 > File size detected as  191 bytes
#> 
#> ℹ 2022-12-19 18:18:57 > File size detected as  23 bytes
#> 
#> Writing to pin 'small-numbers'

^{Created on 2022-12-19 with reprex v2.0.2}

Is there a straightforward way for me to suppress this messaging? An option? I looked around but didn't find it quickly. I think it is getting too noisy, with the pins messages and the googleCloudStorageR messages together.

The second is about how I am supposed to use prefix and delimiter to get to a directory-like mode for listing objects. I can use prefix to, for example, find all the files that belong to one pin:

library(pins)
googleCloudStorageR::gcs_auth("~/google-pins.json")
b <- board_gcs("pins-dev")
googleCloudStorageR::gcs_list_objects(
  bucket = b$bucket,
  prefix = "small-numbers"
)
#>                                                      name      size
#> 1           small-numbers/20221220T011807Z-4d1eb/data.txt 189 bytes
#> 2  small-numbers/20221220T011807Z-4d1eb/small-numbers.rds  61 bytes
#> 3           small-numbers/20221220T011808Z-c3943/data.txt 191 bytes
#> 4 small-numbers/20221220T011808Z-c3943/small-numbers.json  23 bytes
#> 5           small-numbers/20221220T011855Z-4d1eb/data.txt 189 bytes
#> 6  small-numbers/20221220T011855Z-4d1eb/small-numbers.rds  61 bytes
#> 7           small-numbers/20221220T011856Z-c3943/data.txt 191 bytes
#> 8 small-numbers/20221220T011856Z-c3943/small-numbers.json  23 bytes
#>               updated
#> 1 2022-12-20 01:18:08
#> 2 2022-12-20 01:18:08
#> 3 2022-12-20 01:18:09
#> 4 2022-12-20 01:18:09
#> 5 2022-12-20 01:18:55
#> 6 2022-12-20 01:18:56
#> 7 2022-12-20 01:18:57
#> 8 2022-12-20 01:18:57

^{Created on 2022-12-19 with reprex v2.0.2}

However, when I try to use delimiter = "/", that throws everything away, since they all have / in them:

library(pins)
googleCloudStorageR::gcs_auth("~/google-pins.json")
b <- board_gcs("pins-dev")
googleCloudStorageR::gcs_list_objects(
  bucket = b$bucket,
  prefix = "small-numbers",
  delimiter = "/"
)
#> ℹ 2022-12-19 18:22:56 > No objects found
#> data frame with 0 columns and 0 rows

^{Created on 2022-12-19 with reprex v2.0.2}

I want to get something more like a list of directories. I can work around when I know the pin name, but I would like to get pin_list() working, largely so we can support the new workflow outlined in #692 for GCS. You can check out how that happens for Azure and AWS.

MarkEdmondson1234 · 2022-12-20T06:27:20Z

suppressMessages() should do it for the upload feedback. If it needs more fine grain control it could be conditional on options(googleAuthR.verbose = 4) - the default is 3, 4 will suppress all messages.

The objects aren't really in folders more just names with / in them, so listing folders isn't directly supported. If folder structure is important perhaps make new buckets?

Prefix is only to filter results coming back that begin with x, so can do pseudo "list a folder" with "everything that starts with "folder/"

juliasilge · 2022-12-20T21:48:46Z

@MarkEdmondson1234 it looks like googleCloudStorageR::gcs_get_object() does not respect googleAuthR.verbose:

withr::with_options(
  list(googleAuthR.verbose = 4),
  {
    googleCloudStorageR::gcs_auth("~/google-pins.json")
    googleCloudStorageR::gcs_get_object(
      object_name = "big-numbers/20221220T011808Z-b89e8/big-numbers.json",
      bucket = "pins-dev",
      saveToDisk = tempfile()
    )
  }
)
#> ℹ Downloading big-numbers/20221220T011808Z-b89e8/big-numbers.json
#> ✔ Saved big-numbers/20221220T011808Z-b89e8/big-numbers.json to /var/folders/hv/…
#> 
#> [1] TRUE

^{Created on 2022-12-20 with reprex v2.0.2}

I believe other functions like googleCloudStorageR::gcs_upload() and googleCloudStorageR::gcs_list_objects() do respect the option. Would you like me to open an issue? I will use suppressMessages() for that one function for now.

juliasilge · 2022-12-20T21:52:54Z

I'm going to keep pin_list() turned off for now, because it looks like we don't have the same support for handling the flat namespace that we do in Azure and AWS. Since we don't have pin_list(), that means we don't have the ability to make a manifest file. If we get requests for this, we can look into our options for implementing pin_list().

juliasilge · 2022-12-20T22:48:14Z

@MarkEdmondson1234 do you have the bandwidth to do a review of this PR and see if you have any suggestions for better use of the GCS API?

juliasilge · 2022-12-20T22:50:07Z

R/board_gcs.R

+  rlang::check_installed("sodium")
+  path_to_encrypted_json <- fs::path_package("pins", "secret", "pins-gcs-testing.json")
+  raw <- readBin(path_to_encrypted_json, "raw", file.size(path_to_encrypted_json))
+  pw <- Sys.getenv("PINS_GCS_PASSWORD", "")
+  json <- sodium::data_decrypt(
+    bin = raw,
+    key = sodium::sha256(charToRaw(pw)),
+    nonce = secret_nonce()
+  )
+  googleCloudStorageR::gcs_auth(json_file = rawToChar(json))


I ended up taking the approach outlined in the gargle vignette "Managing tokens securely", and it seems to be working great!

Nice solution for local testing

juliasilge · 2022-12-20T22:51:30Z

R/board_gcs.R

+pin_list.pins_board_gcs <- function(board, ...) {
+  NA
+}


Like I discussed earlier in this PR, no pin_list() for the time being because we don't have good tooling for handling the flat namespace in a bucket. This is different from AWS and Azure. We can try coming back to this later if it is a high priority issue for folks.

I guess to enable it this would need to download a list of all objects, then parse through it locally for those ending with /. Its doable, but could get slow if a very large (10k+) number of objects

You can see how we handle that for AWS and Azure. Both of those rely on underlying functionality already available in paws.storage and AzureStor (ways to identify directory-like structure, common prefixes, etc). I don't think I want to move forward with writing that parsing code here in pins for now; maybe we can collaborate in the future on adding features like this to googleCloudStorageR if it is a high priority for users.

Why can't we use gcs_list_objects() plus a prefix? Because it returns all objects "recursively"?

Yes, that's right:

library(pins) googleCloudStorageR::gcs_auth("~/google-pins.json") b <- board_gcs("pins-testing") googleCloudStorageR::gcs_list_objects(bucket = b$bucket) #> name size #> 1 big-numbers/20230110T204627Z-b89e8/big-numbers.json 46 bytes #> 2 big-numbers/20230110T204627Z-b89e8/data.txt 187 bytes #> 3 small-numbers/20230110T204626Z-4d1eb/data.txt 189 bytes #> 4 small-numbers/20230110T204626Z-4d1eb/small-numbers.rds 61 bytes #> 5 small-numbers/20230110T204628Z-c3943/data.txt 191 bytes #> 6 small-numbers/20230110T204628Z-c3943/small-numbers.json 23 bytes #> 7 test-kcH7P3OJjX/20230110T205041Z-6e4eb/data.txt 192 bytes #> 8 test-kcH7P3OJjX/20230110T205041Z-6e4eb/test-kcH7P3OJjX.rds 44 bytes #> updated #> 1 2023-01-10 20:46:28 #> 2 2023-01-10 20:46:28 #> 3 2023-01-10 20:46:26 #> 4 2023-01-10 20:46:27 #> 5 2023-01-10 20:46:29 #> 6 2023-01-10 20:46:29 #> 7 2023-01-10 20:50:41 #> 8 2023-01-10 20:50:41 googleCloudStorageR::gcs_list_objects(bucket = b$bucket, prefix = "big-numbers") #> name size #> 1 big-numbers/20230110T204627Z-b89e8/big-numbers.json 46 bytes #> 2 big-numbers/20230110T204627Z-b89e8/data.txt 187 bytes #> updated #> 1 2023-01-10 20:46:28 #> 2 2023-01-10 20:46:28

^{Created on 2023-01-12 with reprex v2.0.2}

In the packages for AWS and Azure, there is existing support for identifying directory-like structures or to find "common prefixes" but googleCloudStorageR doesn't have that as of today.

juliasilge · 2022-12-20T22:52:33Z

R/board_gcs.R

+  paths <- fs::path_split(unique(fs::path_dir(resp$name)))
+  version_from_path(map_chr(paths, ~ .x[[length(.x)]]))


This is what I did to deal with the flat namespace situation; suggestions welcome for a nicer option!

There is the concept of object versioning in GCS, does this match what pin_versions does? https://cloud.google.com/storage/docs/object-versioning

No, one of the ideas of pins is to provide versioning, even if the data practitioner is not allowed to turn on GCS versioning because of, say, the bucket retention policy.

juliasilge · 2022-12-20T22:53:54Z

R/board_gcs.R

+gcs_download <- function(board, key) {
+  path <- fs::path(board$cache, key)
+  if (!fs::file_exists(path)) {
+    suppressMessages(googleCloudStorageR::gcs_get_object(


This one function doesn't seem to respect googleAuthR.verbose so for now we'll use suppressMessages().

I guess its the use of the cli:: message bar - yes please open an issue as should be a quick fix

Just opened here: cloudyr/googleCloudStorageR#170

MarkEdmondson1234 · 2022-12-21T09:35:32Z

R/board_gcs.R

+  path
+}
+
+gcs_file_exists <- function(board, name) {


I would do this via gcs_get_object("obj_name", meta=TRUE) and then tryCatch(http_404 = FALSE) - will be much quicker and direct only inspecting HTTP headers and not content. Custom error types are supported in the HTTP responses to help this workflow. Listing objects could be slow for large amount of pins

An example here for a pull request I did for targets a while ago https://github.com/MarkEdmondson1234/targets/blob/8204e4268553a2680f5d5ff3b0fc006b0c40d45a/R/utils_gcp.R#L7-22

Ah, I see what you're saying! Hmmm...

It's possible this function could have a better name, but it's not always looking for a specific object; sometimes it is looking for a directory-like thing. Given the information at hand when calling, for example, pin_exists(), I think we do need to list the objects in the bucket rather than get a specific object.

hadley

LGTM!

R/board_gcs.R

hadley · 2023-01-10T22:18:07Z

R/board_gcs.R

+pin_list.pins_board_gcs <- function(board, ...) {
+  NA
+}


Why can't we use gcs_list_objects() plus a prefix? Because it returns all objects "recursively"?

hadley · 2023-01-10T22:19:54Z

R/board_gcs.R

+    prefix = paste0(board$prefix, dir, "/")
+  )
+
+  if (nrow(resp) == 0) {


I think this is probably not needed unless when nrow() is a 0, there's no name column.

Yep, that is the situation:

library(pins) googleCloudStorageR::gcs_auth("~/google-pins.json") b <- board_gcs("pins-testing") resp <- googleCloudStorageR::gcs_list_objects( bucket = b$bucket, prefix = paste0(b$prefix, "cats-and-dogs", "/") ) #> ℹ 2023-01-12 10:50:27 > No objects found resp #> data frame with 0 columns and 0 rows

^{Created on 2023-01-12 with reprex v2.0.2}

R/board_gcs.R

Co-authored-by: Hadley Wickham <[email protected]>

github-actions · 2023-01-28T00:06:41Z

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

juliasilge added 3 commits December 19, 2022 17:59

First stab at GCS board

9cf74ba

Document, update DESCRIPTION + pkgdown

a87bd7d

Tests, plus encrypted service account keys JSON

f8fe5ba

juliasilge added 2 commits December 20, 2022 14:49

Ignore my JSON file

70b0866

No manifest for GCS board, try to turn off noisy messages

835952e

juliasilge added 2 commits December 20, 2022 15:32

Update NEWS and pkgdown

6a509c4

Update docs

76973ae

juliasilge marked this pull request as ready for review December 20, 2022 22:47

juliasilge commented Dec 20, 2022

View reviewed changes

MarkEdmondson1234 reviewed Dec 21, 2022

View reviewed changes

juliasilge mentioned this pull request Jan 10, 2023

gcs_get_object() doesn't respect googleAuthR.verbose cloudyr/googleCloudStorageR#170

Open

juliasilge requested a review from hadley January 10, 2023 21:26

hadley approved these changes Jan 10, 2023

View reviewed changes

juliasilge and others added 4 commits January 12, 2023 10:37

Apply suggestions from code review

a919975

Co-authored-by: Hadley Wickham <[email protected]>

Merge branch 'main' into google-cloud-storage

8afce68

Apply suggestions from code review

3677308

No more default bucket

a89c865

juliasilge merged commit 08d7dad into main Jan 13, 2023

juliasilge deleted the google-cloud-storage branch January 13, 2023 19:18

github-actions bot locked and limited conversation to collaborators Jan 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new `board_gcs()` for Google Cloud Storage #695

Add new `board_gcs()` for Google Cloud Storage #695

juliasilge commented Dec 20, 2022

juliasilge commented Dec 20, 2022

MarkEdmondson1234 commented Dec 20, 2022

juliasilge commented Dec 20, 2022

juliasilge commented Dec 20, 2022

juliasilge commented Dec 20, 2022

juliasilge Dec 20, 2022

MarkEdmondson1234 Dec 21, 2022

juliasilge Dec 20, 2022

MarkEdmondson1234 Dec 21, 2022

juliasilge Jan 10, 2023

hadley Jan 10, 2023

juliasilge Jan 12, 2023

juliasilge Dec 20, 2022

MarkEdmondson1234 Dec 21, 2022

juliasilge Jan 10, 2023

juliasilge Dec 20, 2022

MarkEdmondson1234 Dec 21, 2022

juliasilge Jan 10, 2023

MarkEdmondson1234 Dec 21, 2022

MarkEdmondson1234 Dec 21, 2022

juliasilge Jan 10, 2023

hadley left a comment

hadley Jan 10, 2023

hadley Jan 10, 2023

juliasilge Jan 12, 2023

github-actions bot commented Jan 28, 2023

		paths <- fs::path_split(unique(fs::path_dir(resp$name)))
		version_from_path(map_chr(paths, ~ .x[[length(.x)]]))

Add new board_gcs() for Google Cloud Storage #695

Add new board_gcs() for Google Cloud Storage #695

Conversation

juliasilge commented Dec 20, 2022

juliasilge commented Dec 20, 2022

MarkEdmondson1234 commented Dec 20, 2022

juliasilge commented Dec 20, 2022

juliasilge commented Dec 20, 2022

juliasilge commented Dec 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hadley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 28, 2023

Add new `board_gcs()` for Google Cloud Storage #695

Add new `board_gcs()` for Google Cloud Storage #695