It was based on the method used in the FTP FileSystem which used a runtime exception. Should I use IOException then? For example if it throws an IOException at "exists" method, the API should be changed, I think that's why they used FTPException. In this case, should I just solve the exception locally? or throwing the SFTPException?
Yikes; I didn't know it did that. Yes, please throw IOException and not RuntimeException. Applications won't expect the latter.
Done, it extends FSInputStream. Should I modify something in order to do it package-private?
Top-level classes without a visibility modifier are package-private, i.e. are not visible to classes outside the package. No other classes should be creating SFTPInputStream instances, right?
I work with hadoop 0.19, which defines this methods, I prefer specifying it as deprecated.
I see. As a new feature, the earliest branch to which it should be committed is 0.21, so the @Override annotations should fail. If you'd like to attach patches for 0.19 and/or 0.20 on this issue for others to use in their own deployments, that's great, but the version committed to the mainline should not include these as they should have no callers.
> Any particular reason for not supporting setting the working directory?
It is the same problem with FTPFileSystem: "Directory on the server is changed to the parent directory of the file. The FTP client connection is closed when close() is called on the FSDataInputStream."
That makes sense. Since the working directory can be a Path member in the FileSystem handle, you could make all paths relative to that path instead of the default directory, right? Applications referencing generic filesystems might use the working directory abstraction, which would have unexpected results with this FileSystem. If FTPFileSystem has the same quirk and you've documented it, it's OK if you want to leave it that way, but I'd lean towards supporting it. Applications that deal with generic FileSystems- like DistCp- usually end up creating absolute paths to avoid edge cases like this one, but it would be nice if that were unnecessary. It's up to you.
How do you plan to test this, incidentally? I'd recommend having some test.* properties that, if set, would let you sftp back in to localhost or another named host.
The unit tests should not require the user running the tests to have an ssh login on the host machine, neither should it be necessary to configure such a user. If jsch has a clever way to run its unit tests without actually performing these checks- perhaps in its development branch- that would be hugely, hugely preferred. If that's not possible... then like KFS, there's not much to be done to prevent regressions.
Please create a unified patch with the ivy changes that applies to 0.21. Once that's done, you can submit the patch to automated testing using the "Submit Patch" link to the left.