filesystem_spec
filesystem_spec copied to clipboard
Change sftp rename from posix_rename() to rename()
See discussion on https://stackoverflow.com/questions/70658956/paramiko-sftp-file-renaming-oserrorextended-request-not-supported/70668235. The posix_rename() appears to limit the usage of the SFTPFileSystem.rename() method to sFTP servers that run on Posix-like servers.
Copy is not in the core spec, either, and I think that there is less support for that than for posix_rename(). It does not help with my issue - I get a NotImplementedError(). See Martin's comment on https://stackoverflow.com/questions/28089821/how-can-i-copy-duplicate-a-file-to-another-directory-using-sftp. It would probably best to expose the core rename() function, which works reliably, as a new method, no?
I don't suppose paramiko has any way to ascertain which capabilities the remote server has?
The major problem with this change is that, AFAIK regular rename
errors when the target file (or the directory) already exists. Let's initially add a test for this (fs.touch(file_a); fs.touch(file_b); fs.move(file_a, file_b)
), and then discuss how to probably solve it (another approach might be checking whether the target file is already there and removing it and then doing the rename, but this has to be done atomically which isn't easy).
rename errors when the target file (or the directory) already exists
That's not quite terrible! In fact, some would assume that is the case, but I think other implementations do indeed clobber.
but this has to be done atomically which isn't easy).
ideally yes, but many other operations (such as copy then delete) are not.
I agree with Martin that an error is the expected response when a file with the new name already exists. I would not want it quietly overwritten. In any case, I am not clear what the direction is for this. I can't work on a test right away. It is not urgent because I have a workaround (fs.ftp.rename()
).
I don't suppose paramiko has any way to ascertain which capabilities the remote server has?
No, Paramiko does not parse the extensions supported by the server from the SFTP "INIT" packet. See the BaseSFTP._send_version
– it reads only the version number and ignores the rest of the packet.
I agree with Martin that an error is the expected response when a file with the new name already exists.
I would expect it to override, just like every other filesystem we have. I understand that this is some sort of an SFTP restriction (that is why we have 2 implementations in the sshfs), so if there is no easy workarounds I guess it would be fine as is.
I would just like to note again that we would be changing behavior for existing (if any?) users of sshfs.mv()
. I don't use it so I so I don't really plan to strongly object (though would love if there were a compromise, e.g try/excepting regular posix_rename
and if it fails with OSError
just using rm
+ rename
(even if it is not atomic)).
Is there any chance of a consensus here?