search

cloudyr / googleCloudStorageR

Google Cloud Storage API to R
https://code.markedmondson.me/googleCloudStorageR
Other
104 stars 29 forks source link

Fix gcs_copy_object() to use bucketLevel permissions #132

Open MarkEdmondson1234 opened 3 years ago

MarkEdmondson1234 commented 3 years ago

@MarkEdmondson1234 Thanks for the Hotfix. But i Think you missed the gcs_copy_object() function. There should also be a parametervalue "bucketLevel". I tried it with the latest github commit, but it didn't work. and it says

Request Status Code: 400 Error: API returned: Invalid string value: 'bucketLevel'. Allowed values: [authenticatedread, bucketownerfullcontrol, bucketownerread, private, projectprivate, publicread]

As long as the update isn't available in CRAN. Can you please post a snippet which removes the ACLs from the call to get this work?

Originally posted by @Pit-Storm in https://github.com/cloudyr/googleCloudStorageR/issues/111#issuecomment-714367836

MarkEdmondson1234 commented 3 years ago

@Pit-Storm best tracked in a new issue

It will involve adding a blank ACL object as shown in this commit https://github.com/cloudyr/googleCloudStorageR/commit/37f922cc5ce96a3bfdbcdc4f6c0466991ebe18bc to the gcs_copy_object() code:

https://github.com/cloudyr/googleCloudStorageR/blob/276fe3a89d8bf20878860f3665877b3109225535/R/objects.R#L378-L422

MarkEdmondson1234 commented 3 years ago

@Pit-Storm what example code are you using to trigger your error? From the looks of it not specifying the ACL should respect that existing objects ACL level.

Pit-Storm commented 3 years ago

I fixed it by setting predefinedACL to NULL.

Aren't you think that this is little inconvenient with respect to the upload function?

Kind reagrds

  1. Oktober 2020 11:26, "Mark" <notifications@github.com (mailto:notifications@github.com?to=%22Mark%22%20notifications@github.com)> schrieb: @Pit-Storm (https://github.com/Pit-Storm) what example code are you using to trigger your error? From the looks of it not specifying the ACL should respect that existing objects ACL level.

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/cloudyr/googleCloudStorageR/issues/132#issuecomment-716454982), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AIBQ47XUB2G4SPH2SHHJKXDSMVFEDANCNFSM4S7FTZXQ).

MarkEdmondson1234 commented 3 years ago

I'm a bit confused as the default is NULL. May I ask for the example code you used and the sessionInfo(), it could be that its just fixed on GitHub?

Pit-Storm commented 3 years ago

I think I made some missunderstandable comments.

In the first place, I used predefinedACL="bucketLevel" because I knew the setting from the upload object. I wasn't trying without the predefinedACL parameter. After it didn't work, I checked the docs for it. There wasn't any bucketLevel or NULL setting for this parameter mentioned. For this reason i opend the ticket. After that, I saw that there had been a discussion on that and your suggestion to set the ACL to NULL when doing the call. So I tried predefinedACL=NULL. I didn't check the dfault setting.

As far as I can see that issue it is a peace about docs and a peace about convenient parameter settings. In my eyes it is a little confusing why one has to be set the predefinedACL on the upload function explicitly if the bucketLevelACLs are enabled and on the other side one is able to use a default parameter setting with the copy function. But I'm not sure if I missunderstood something in general.

Kind regards

PS: I'm not able to provide a full working code example for now, because I'm not at my work machine.

MarkEdmondson1234 commented 3 years ago

Makes sense, yes it can be changed to be more consistent.