r-spatial / rgee

Google Earth Engine for R
https://r-spatial.github.io/rgee/
Other
677 stars 146 forks source link

Fix uniform bucket access issue #203

Closed jsocolar closed 2 years ago

jsocolar commented 2 years ago

Can close #202, but a bit of attention is required prior to merging (see below)

This PR adds a predefinedAcl argument that is passed through *_as_ee, though local_to_gcs, and then to googleCloudStorageR::gcs_upload.

It additionally updates tests to cover this case. Some of these tests rely on a bucket with uniform access, currently hardcoded in testthat.R as

# Define your own GCS bucket with fine-grained access to save 
# intermediate files. ALERT!!: After test finished all the files 
# inside the bucket will be deleted.
gcs_bucket_f <- function(){
  "rgee_dev"
}

# Define your own GCS bucket with uniform access to save 
# intermediate files. ALERT!!: After test finished all the files 
# inside the bucket will be deleted.
gcs_bucket_uniform_f <- function(){
  "rgee_dev_uniform"
}

Prior to merging @csaybar might want to either delete the above function and the new tests in test-upload.R or (preferably?) set up this bucket in your own testing infrastructure so that these tests can run. I thought it would be a good idea to get test coverage here so that CRAN will help googleCloudStorageR to avoid changes in the predefinedAcl argument that could break things for rgee.

I have not verified that the contents of this new uniform-access bucket will get properly deleted when the tests finish. This needs to be verified and updated if necessary.

I've bumped the version number of googleCloudStorageR in Suggests from 5.1 to 6.0 because I'm too lazy to verify which version number first included the predefinedAcl argument.

I've also tweaked local_to_gcs so that if quiet = FALSE and googleCloudStorageR::gcs_upload errors repeatedly, the user will see the error returned on the final try (seeing this error will help the user recognize if they need to change the predefinedAcl argument).

csaybar commented 2 years ago

Hi @jsocolar, thank you for helping us solve this. I just made some minor changes to your code. I added a cat rather than having to run googleCloudStorageR::gcs_upload again :). I can ensure that the changes work great in Windows and Ubuntu :) https://github.com/r-spatial/rgee/blob/128014c34c6776b62547f9c5d200be4ec7632771/R/utils-upload.R#L67