-
Notifications
You must be signed in to change notification settings - Fork 266
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
Support to save/load files to/from sshfs-win volumes #1449
base: main
Are you sure you want to change the base?
Conversation
This issue could also be relevant, but i'm not sure: https://forum.image.sc/t/qupath-project-didnt-save-images-and-annotations-are-wrong/61639/8 Depends how their network drive was mounted! |
Thanks @carlocastoldi The change looks a bit scary to me - although it's possible the existing behavior is already a bit scary. I see the javadocs state the following regarding
Nevertheless, I still feel uneasy about explicitly deleting before attempting any copy or move.... since 'may not be atomic' still gives me some hope that we won't end up in some unfortunate state. Corrupt data files was previously a somewhat common complaint, which has reduced a lot over recent releases. Is there any easy way for us to replicate the issue? Or, if not, could you give the full stack trace for what exception you get without these changes? I wonder whether creating a method for copy/move that attempts the old behavior, and reverts to the new one if that fails for some reason. |
If atomicity of the operation is what worries you, i could implement a method that does it safely, very similarly to what you do with
The "problem" is that there is no exception thrown. It just fails on step 3 and/or 4 because
Not that I know of. You have to connect to an SFTP server with windows to do this. Finally, i think Regarding the |
I agree, this feels a bit aggressive to me. If the existing copy may not be atomic, I'd rather try to implement an atomic version than to switch to a "definitely not atomic" version.
This sounds good to me, it's important for obvious reasons that user data operations are done as safely as possible. |
I am afraid that copy operations were never guaranteed to be atomic in mainstream OSes. But I agree that this should be something where chance has no room to play
Alright then, I'll work on it. |
083300b
to
b95761e
Compare
Recently my laboratory had to move all its files from a Samba storage server to a linux one, accessible only through SFTP/SSH.
For this reason my colleagues' Windows PCs were forced to use
sshfs-win
in order to mount the remote storage when loading images from QuPath.Everything seem to work as intended, however saving projects (and probably other files too) is problematic. This is not a problem of QuPath itself, but rather a know problem of
sshfs-win
not (yet) supporting Windows'GetFileInformationByHandle
to retrieve file identifiers.These identifiers are actively used by Java's Windows' filesystem provider to check whether two files are the same when requesting for a
copy()
ormove()
operation.All this PR does is to use
Files.deleteIfExists()
instead of callingcopy()
andmove()
withStandardCopyOption.REPLACE_EXISTING
. Semantically the result is the same.I know that the issue is not on QuPath's side, however since the fix is very small on yourside i was hoping you would accept this PR anyway. It may come handy to other people in similar scenarios that don't know how to takle the problem