file download tweaks for multiple paths

[sckott]

two changes here:

1. change single & to double && as I assume you intended to end up with a single boolean in the if block?
2. and change fs::is_dir(new_path) to all(fs::is_dir(new_path)) so that i can pass in multiple paths to s3_file_download. perhaps this is the wrong behavior? I don't know the code here well enough to know

With current version of the package

library(s3fs)
library(dplyr)
s3paths <- s3_dir_info("s64-test-22") %>% pull(uri) # length is 4
dfiles <- replicate(n = 4, tempfile())
s3_file_download(
    path = s3paths, 
    new_path = dfiles
)
#> Error in if (length(new_path) == 1 & fs::is_dir(new_path)) {: the condition has length > 1

With this change

library(s3fs)
library(dplyr)
s3paths <- s3_dir_info("s64-test-22") %>% pull(uri) # length is 4
dfiles <- replicate(n = 4, tempfile())
s3_file_download(
    path = s3paths, 
    new_path = dfiles
)
#> [1] "/var/folders/qt/fzq1m_bj2yb_7b2jz57s9q7c0000gp/T//Rtmpagnkcx/filecfc62b59ced4"
#> [2] "/var/folders/qt/fzq1m_bj2yb_7b2jz57s9q7c0000gp/T//Rtmpagnkcx/filecfc61519d688"
#> [3] "/var/folders/qt/fzq1m_bj2yb_7b2jz57s9q7c0000gp/T//Rtmpagnkcx/filecfc65353858a"
#> [4] "/var/folders/qt/fzq1m_bj2yb_7b2jz57s9q7c0000gp/T//Rtmpagnkcx/filecfc6106451c3"

[DyfanJones]

@sckott Thanks for the PR. This looks good to me :) Please feel free to add your contribution to the NEWS.md :)

[sckott]

Thanks, will do

[sckott]

Okay, I added news item, let me know if i should change it at all.

Also, I tried to run tests, but ran into some issues that I don't think are related to my changes, so gave up on that. Seems like the tests didn't run in CI either Reason I ask is because this change I think affects s3_file_download_async and the copy functions, and I wanted to make sure I didn't break thoes. Can you test?

[DyfanJones]

@sckott ah thanks for that, I didn't realise the tests weren't working correctly. I will look into it :)

[DyfanJones]

Ran this locally and it seems to be working. Not 100% sure why this PR isn't picking up the environmental variables

• AWS_ACCESS_KEY_ID
• AWS_SECRET_ACCESS_KEY
• AWS_REGION
• AWS_S3_BUCKET_NOT_VERSIONED
• AWS_S3_BUCKET_VERSIONED

Possibly it is trying to take environment variables from your fork 🤔 but I am not sure. I will merge for now and check with subsequent PRs

[DyfanJones]

@sckott Seems to be fine now https://github.com/DyfanJones/s3fs/actions/runs/8233733113/job/22513849085#step:5:178 😄

[sckott]

Not 100% sure why this PR isn't picking up the environmental variables

i'm pretty sure that env vars aren't available in PRs

Thanks for merging!