-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes 192: Use file.copy for cross-disk flexibility in HYDAT download #205
Fixes 192: Use file.copy for cross-disk flexibility in HYDAT download #205
Conversation
Replaced file.rename with file.copy to handle situations where the temp folder and the destination are on different physical disks (e.g. Docker container with different mount points for the tempdir and the destination).
Hi there. Thanks for the contribution. Can you please provide an example where this is an issue? Also how do you think that we might test this? |
Actually testing this could an issue which is my fault for including too much into that download function. I've made #206 for that. But I would really like to see how this actually fails. So if you could provide a Dockerfile that can actually trigger this issue that would be great. |
This issue occurs when files are being renamed across different partitions. In my scenario, the rename action fails due to a cross-device link error. This is a limitation of the function, as detailed in the docs for Though it's probably possible to write a test for this, it might not be appropriate. After reading through those docs, I see that I need to update the call to One-liner that reproduces the issue with docker:$ docker run \
-v /tmp/some_folder:/root \
rocker/r-ver \
Rscript -e 'install.packages("tidyhydat"); library(tidyhydat); download_hydat(ask = FALSE)'
<snip />
✔ Downloading HYDAT to /root/.local/share/tidyhydat
• Downloading new version of HYDAT created on 2024-07-18
|======================================================================| 100%
• Extracting HYDAT
✖ HYDAT not successfully downloaded
Error in hy_src(hydat_path) :
No Hydat.sqlite3 found at /root/.local/share/tidyhydat. Run download_hydat() to download the database.
Calls: download_hydat -> hy_check -> hy_src
In addition: Warning message:
In file.rename(list.files(tempdir, pattern = "\\.sqlite3$", full.names = TRUE), :
cannot rename file '/tmp/RtmplUpKBw/extracted/Hydat.sqlite3' to '/root/.local/share/tidyhydat/Hydat.sqlite3', reason 'Invalid cross-device link'
Execution halted
$ One-line that completes successfully using the branch:$ docker run \
-v /tmp/some_folder:/root \
rocker/r-ver \
Rscript -e 'install.packages("devtools"); devtools::install_github("thompson-aquatic/tidyhydat", ref="enhance/flexible-hydat-file-copy"); library(tidyhydat); download_hydat(ask = FALSE)'
<snip />
✔ Downloading HYDAT to /root/.local/share/tidyhydat
• Downloading new version of HYDAT created on 2024-07-18
|======================================================================| 100%
• Extracting HYDAT
★ HYDAT successfully downloaded
$ |
Ok this is awesome. Thanks for this contribution. I am going to try to fix up a couple things before I ship this to CRAN. LMK if you are needing a CRAN release soon for your use cas.e |
Can also please add a NEWS item and your handle for attribution? |
added some backticks
No problem -- happy to help. I can continue to work around using the branch until a new release comes out, so no rush on my account. I've added to NEWS.md. |
Replaced file.rename with file.copy to handle situations where the temp folder and the destination are on different physical disks (e.g. Docker container with different mount points for the tempdir and the destination).