search

rstudio / pins-r

Pin, discover, and share resources
https://pins.rstudio.com
Other
312 stars 63 forks source link

add ... to s3_upload_yaml to fix issue noted in #625 #652

Closed fh-mthomson closed 2 years ago

fh-mthomson commented 2 years ago

Fixes issue discussed in https://github.com/rstudio/pins-r/issues/625#issuecomment-1242558838

juliasilge commented 2 years ago

Thank you so much for this @fh-mthomson! 🙌

Can I make sure I am understanding this super clearly? This change will make it so both the main file (such as .rds or .csv) and the data.txt are uploaded with the same arguments/options for S3 PutObject? That seems like the right behavior to me, and it's hard to imagine a situation where that isn't what folks want.

fh-mthomson commented 2 years ago

Exactly! Since the bucket to which I'm writing has SSE enabled, trying to write the data.txt file resulted in permissions errors when trying to call put_object() but without the ServerSideEncryption specification available.

So, I think it's reasonable to expect that S3-level options should be passed through to all instances of put_object() since each call is inherently within the same bucket/prefix.

github-actions[bot] commented 2 years ago

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.