[
https://issues.apache.org/jira/browse/VFS-422?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13398482#comment-13398482
]
Gary D. Gregory commented on VFS-422:
-------------------------------------
Hi Benjamin:
Thank you for providing another patch.
Let's discuss:
- Meta: The patch format you supplied was painful for me to use. I had to cut
and paste individual fragments for each file and remove the {{a/}} and {{b/}}
from the paths. Can you use one plain old .diff file in the future please?
- The above may mean I misapplied the patch because running "mvn clean test", I
get:
{noformat}
Tests in error:
testExternalChannel(org.apache.commons.vfs2.provider.sftp.test.ProviderSftpExternalChannelTest)
{noformat}
- Session type: Why is this not an enum? This sure seems like the perfect
application of an enum. The implementation of
{{com.jcraft.jsch.CommonsVFSChannelHelper.getChannel(Session, AtomicLong,
String)}} can then be use a switch instead of a bunch of {{if}}s. Also, with
enumns, no chance of a typo creating a bug.
- {{com.jcraft.jsch.CommonsVFSChannelHelper.getChannel(Session, AtomicLong,
String)}}
Why is a runtime exception thrown here:
{code:java}
@Override
public void close() {
if (!close)
counter.decrementAndGet();
super.close();
throw new RuntimeException("Closed call");
}
{code}
- Always use blocks, for example:
{code:java}
if (!close) {
counter.decrementAndGet();
}
{code}
Or, in the case of the goofy VFS style:
{code:java}
if (!close)
{
counter.decrementAndGet();
}
{code}
- {{com.jcraft.jsch.CommonsVFSChannelHelper.getChannel(Session, AtomicLong,
String)}} instance initializers
Do we really care to track how many objects are instantiated? Would the
increment be better in connect() overrides? This seems to match up better with
the decrement in close(). What am I missing? Some code comments would help here
for the non-Jsch guru.
> Allows to create other channels in SftpFileSystem
> -------------------------------------------------
>
> Key: VFS-422
> URL: https://issues.apache.org/jira/browse/VFS-422
> Project: Commons VFS
> Issue Type: Improvement
> Affects Versions: 2.0
> Environment: Any
> Reporter: Benjamin Piwowarski
> Attachments:
> 0001-SftpFileSystems-allows-opening-external-channels.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
> In the software I am writing, I need to execute commands when the filesystems
> "allows" it, i.e. local or via ssh (i.e. sftp filesystem).
> For a local filesystem, I can easily do this, but for Sftp, there is no way
> to get a channel different than the SFTP on, so it would be great if other
> channels could be open.
> I could submit a patch that would:
> 1) Allows a call to session.openChannel(String type) [following getChannel()]
> 2) Overwrite isReleaseable to keep the filesystem open if some external
> channels are open
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
