localfs: add link/symlink/islink methods

[skshetry]

This PR adds link/symlink APIs to create hardlink/symlink, and islink to check for symlinks.
The APIs mirror what we have in os and os.path.

There are some open questions about APIs. In DVC, we use hardlink(), symlink() and is_symlink() APIs which I find to be much better, but I went with other APIs to be consistent with python. Python has not been very consistent though (Path.is_symlink(), Path.symlink_to, Path.hardlink_to, etc).

Also, do we need these APIs in AbstractFileSystem?

[martindurant]

Do I remember that there was a plan to implement this kind of thing in a filesystem-specific .os.path-like attribute/accessor? (cc @efiop )

Also, do we need these APIs in AbstractFileSystem?

No, I think links are implemented on too few backends.

[skshetry]

Do I remember that there was a plan to implement this kind of thing in a filesystem-specific .os.path-like attribute/accessor? (cc @efiop )

The proposed fs.path only performs path manipulations. @efiop might elaborate further.
The symlink/hardlink do modify the filesystem, so I don't think they fit inside that. islink could be pushed into that if we expand its role in the same way as os.path, but currently, it is strictly path manipulations. islink in this way is similar to fs.isdir/fs.isfile/fs.exists, and os.path.{isdir,isfile,exists} respectively.

[efiop]

@martindurant Saugat is right, fs.path is only for path manipulation. link/symlink/islink don't belong there.

[efiop]

Waiting for @martindurant final comments on this.

[skshetry]

@efiop @martindurant, any thoughts on API namings?

[martindurant]

I am happy to follow the names in os, as the posix CLI corresponding operations are achieved with flags like -l/L, -s.

Before merging, any thoughts on more adding options for following symbolic links or not? Normally, I feel that rm/unlink (delete) should never follow links, so the option is not needed.

[efiop]

Naming makes sense to me. This is universal enough and in the spirit of FUSE/syscalls/etc. The only one that is a bit special is islink, which is a convenience python method, really, since you can easilly figure the same out from stat or fs.info call (we have islink there already in localfs).

One thing that worries me here is that this is localfs only right now, while all three methods should probably be a part of spec, since multiple filesystems support it (local, ssh, hdfs, probably more). That would again be more in a spirit of FUSE.

Looking forward to @martindurant 's opinion on these things. 🙂

[martindurant]

all three methods should probably be a part of spec

Let's put them here and allow to sit for a while. Implementation in the spec would be just NotImplemented anyway

local, ssh, hdfs, probably more

I don't think so? Does HDFS really?

[efiop]

Let's put them here and allow to sit for a while. Implementation in the spec would be just NotImplemented anyway

Totally fine with that, was just leaving a friendly comment. Another analogy that comes to mind is sign that also seems kinda rare, but is part of the spec. I don't mean to say that we need to overload fs with all sorts of stuff just because there are precedents, just noting. 🙂 It is indeed hard sometimes to tell what should really is generic enough to justify inclusion into as spec. In practical terms, that doesn't make a giant difference for the users, as they can just getattr(fs, "symlink", None) for now.

I don't think so? Does HDFS really?

Yeah, it is a bit messy though https://issues.apache.org/jira/browse/HDFS-245 and kinda limited.

[martindurant]

Perhaps if we get a future request, we could add symbolic=False to the signature of link() as an alias for symlink(), but I didn't think there's any need to complicate things here.

[martindurant]

https://issues.apache.org/jira/browse/HDFS-245

What a thread! Still didn't tell me how to do that from the python API :)