Quantcast

Contribution request: add support for SFtp and SSH for publishing and resolving

classic Classic list List threaded Threaded
38 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Contribution request: add support for SFtp and SSH for publishing and resolving

Daz DeBoer-2
G'day

Currently, the 'maven' and 'ivy' repository implementations only support publishing and resolving via HTTP, HTTPS and local file system. To use SFTP, SSH or any other protocol, users currently need to revert to using a custom Ivy resolver to provide this functionality. Since we've deprecated this mechanism, it would be great to add native support for different protocols to the 'ivy' and 'maven' repositories. Support would be useful for dependency resolution, as well as for publishing via the new publishing plugins.

We're wondering if there's any person or group out there who wants to take this on? The nice thing about this enhancement is that it's relatively self-contained. There will however, be a significant number of new tests required, together with some test infrastructure to allow us to validate these protocols (we already have infrastructure for testing SFtp).

The implementation will likely involve:
  • Adding new RepositoryTransport implementations for SFtp and SSH. (Possibly WebDav too).
  • Improving the way we select a RepositoryTransport implementation based on the repository URL(s).
  • Adding integration tests for resolving from and publishing to both ivy and maven repositories with the new protocols (publishing via new publishing plugins).
If you're seriously interested in taking on this project, please let us know and we can start working together on a design document and discussing implementation possibilities.
Thanks for your consideration.
cheers
Daz
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi
I would be interested in implementing this. I was wondering some time ago if I could use gradle to resolve/publish dependencies from/to S3. If I work on this I could learn how/if that requirement could be implemented in the future.

Where shall I start?


On Mon, Feb 17, 2014 at 12:50 AM, Daz DeBoer <[hidden email]> wrote:
G'day

Currently, the 'maven' and 'ivy' repository implementations only support publishing and resolving via HTTP, HTTPS and local file system. To use SFTP, SSH or any other protocol, users currently need to revert to using a custom Ivy resolver to provide this functionality. Since we've deprecated this mechanism, it would be great to add native support for different protocols to the 'ivy' and 'maven' repositories. Support would be useful for dependency resolution, as well as for publishing via the new publishing plugins.

We're wondering if there's any person or group out there who wants to take this on? The nice thing about this enhancement is that it's relatively self-contained. There will however, be a significant number of new tests required, together with some test infrastructure to allow us to validate these protocols (we already have infrastructure for testing SFtp).

The implementation will likely involve:
  • Adding new RepositoryTransport implementations for SFtp and SSH. (Possibly WebDav too).
  • Improving the way we select a RepositoryTransport implementation based on the repository URL(s).
  • Adding integration tests for resolving from and publishing to both ivy and maven repositories with the new protocols (publishing via new publishing plugins).
If you're seriously interested in taking on this project, please let us know and we can start working together on a design document and discussing implementation possibilities.
Thanks for your consideration.
cheers
Daz

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Daz DeBoer-2
Hey Marcin
That's great. If you want to take a stab at a design spec for supporting an sftp repository transport, that would be a good start. This can go into a new document: /design-docs/repository-transports.md.

For the first story I think it should be sufficient to simply support a 'sftp://host[:port]' type URL, together with the existing user/password credentials. But we should probably start thinking about how we can support other authentication mechanisms as well (for all transports)

We also need to start thinking about how we're going to provide adequate test coverage for different repository transports. Do we want to run all of our remote resolution tests against all remote transports, or choose a subset of tests that would be sufficient for demonstrating a particular transport behaves correctly? We can probably come up with a org.gradle.integtests.fixtures.AbstractMultiTestRunner subclass that can run a set of tests against a defined set of transports.

I'll try to start on a design spec sometime this week, but if you want to dive in and get started, that would be great.
cheers
Daz



On Tue, Feb 18, 2014 at 12:15 PM, Marcin Erdmann <[hidden email]> wrote:
I would be interested in implementing this. I was wondering some time ago if I could use gradle to resolve/publish dependencies from/to S3. If I work on this I could learn how/if that requirement could be implemented in the future.

Where shall I start?


On Mon, Feb 17, 2014 at 12:50 AM, Daz DeBoer <[hidden email]> wrote:
G'day

Currently, the 'maven' and 'ivy' repository implementations only support publishing and resolving via HTTP, HTTPS and local file system. To use SFTP, SSH or any other protocol, users currently need to revert to using a custom Ivy resolver to provide this functionality. Since we've deprecated this mechanism, it would be great to add native support for different protocols to the 'ivy' and 'maven' repositories. Support would be useful for dependency resolution, as well as for publishing via the new publishing plugins.

We're wondering if there's any person or group out there who wants to take this on? The nice thing about this enhancement is that it's relatively self-contained. There will however, be a significant number of new tests required, together with some test infrastructure to allow us to validate these protocols (we already have infrastructure for testing SFtp).

The implementation will likely involve:
  • Adding new RepositoryTransport implementations for SFtp and SSH. (Possibly WebDav too).
  • Improving the way we select a RepositoryTransport implementation based on the repository URL(s).
  • Adding integration tests for resolving from and publishing to both ivy and maven repositories with the new protocols (publishing via new publishing plugins).
If you're seriously interested in taking on this project, please let us know and we can start working together on a design document and discussing implementation possibilities.
Thanks for your consideration.
cheers
Daz


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi
I will have a look around the codebase and start working on the design spec around the end of the week if you won't start on it by then.


On Wed, Feb 19, 2014 at 4:06 AM, Daz DeBoer <[hidden email]> wrote:
Hey Marcin
That's great. If you want to take a stab at a design spec for supporting an sftp repository transport, that would be a good start. This can go into a new document: /design-docs/repository-transports.md.

For the first story I think it should be sufficient to simply support a 'sftp://host[:port]' type URL, together with the existing user/password credentials. But we should probably start thinking about how we can support other authentication mechanisms as well (for all transports)

We also need to start thinking about how we're going to provide adequate test coverage for different repository transports. Do we want to run all of our remote resolution tests against all remote transports, or choose a subset of tests that would be sufficient for demonstrating a particular transport behaves correctly? We can probably come up with a org.gradle.integtests.fixtures.AbstractMultiTestRunner subclass that can run a set of tests against a defined set of transports.

I'll try to start on a design spec sometime this week, but if you want to dive in and get started, that would be great.
cheers
Daz



On Tue, Feb 18, 2014 at 12:15 PM, Marcin Erdmann <[hidden email]> wrote:
I would be interested in implementing this. I was wondering some time ago if I could use gradle to resolve/publish dependencies from/to S3. If I work on this I could learn how/if that requirement could be implemented in the future.

Where shall I start?


On Mon, Feb 17, 2014 at 12:50 AM, Daz DeBoer <[hidden email]> wrote:
G'day

Currently, the 'maven' and 'ivy' repository implementations only support publishing and resolving via HTTP, HTTPS and local file system. To use SFTP, SSH or any other protocol, users currently need to revert to using a custom Ivy resolver to provide this functionality. Since we've deprecated this mechanism, it would be great to add native support for different protocols to the 'ivy' and 'maven' repositories. Support would be useful for dependency resolution, as well as for publishing via the new publishing plugins.

We're wondering if there's any person or group out there who wants to take this on? The nice thing about this enhancement is that it's relatively self-contained. There will however, be a significant number of new tests required, together with some test infrastructure to allow us to validate these protocols (we already have infrastructure for testing SFtp).

The implementation will likely involve:
  • Adding new RepositoryTransport implementations for SFtp and SSH. (Possibly WebDav too).
  • Improving the way we select a RepositoryTransport implementation based on the repository URL(s).
  • Adding integration tests for resolving from and publishing to both ivy and maven repositories with the new protocols (publishing via new publishing plugins).
If you're seriously interested in taking on this project, please let us know and we can start working together on a design document and discussing implementation possibilities.
Thanks for your consideration.
cheers
Daz



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch
In reply to this post by Daz DeBoer-2

On 19 Feb 2014, at 3:06 pm, Daz DeBoer <[hidden email]> wrote:

Hey Marcin
That's great. If you want to take a stab at a design spec for supporting an sftp repository transport, that would be a good start. This can go into a new document: /design-docs/repository-transports.md.

For the first story I think it should be sufficient to simply support a '<a href="sftp://host[:port]'">sftp://host[:port]' type URL, together with the existing user/password credentials. But we should probably start thinking about how we can support other authentication mechanisms as well (for all transports)

That would be good. I would love to make credentials a first-class concept and expose some nice command-line and tooling API mechanisms to provide and manage them.


We also need to start thinking about how we're going to provide adequate test coverage for different repository transports. Do we want to run all of our remote resolution tests against all remote transports, or choose a subset of tests that would be sufficient for demonstrating a particular transport behaves correctly? We can probably come up with a org.gradle.integtests.fixtures.AbstractMultiTestRunner subclass that can run a set of tests against a defined set of transports.

One thing that the current test coverage does is to verify exactly which network requests are being made when. This is really important from a performance point of view. I would not want to lose this coverage if we were to abstract some of the tests away from transport, and any new transports should have a similar kind of coverage. Perhaps we might change the test fixtures to abstract some of the verifications (there’s really only a handful of different interactions we need to check for).


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi



On Thu, Feb 20, 2014 at 8:33 PM, Adam Murdoch <[hidden email]> wrote:

On 19 Feb 2014, at 3:06 pm, Daz DeBoer <[hidden email]> wrote:

Hey Marcin
That's great. If you want to take a stab at a design spec for supporting an sftp repository transport, that would be a good start. This can go into a new document: /design-docs/repository-transports.md.

For the first story I think it should be sufficient to simply support a 'sftp://host[:port]' type URL, together with the existing user/password credentials. But we should probably start thinking about how we can support other authentication mechanisms as well (for all transports)

That would be good. I would love to make credentials a first-class concept and expose some nice command-line and tooling API mechanisms to provide and manage them.


We also need to start thinking about how we're going to provide adequate test coverage for different repository transports. Do we want to run all of our remote resolution tests against all remote transports, or choose a subset of tests that would be sufficient for demonstrating a particular transport behaves correctly? We can probably come up with a org.gradle.integtests.fixtures.AbstractMultiTestRunner subclass that can run a set of tests against a defined set of transports.

One thing that the current test coverage does is to verify exactly which network requests are being made when. This is really important from a performance point of view. I would not want to lose this coverage if we were to abstract some of the tests away from transport, and any new transports should have a similar kind of coverage. Perhaps we might change the test fixtures to abstract some of the verifications (there’s really only a handful of different interactions we need to check for).

Seems like a valid point. Looks like you're talking about specs that are using org.gradle.test.fixtures.maven.HttpResource.expect...() methods. We could abstract that to something like RemoteResource and use that to run all these tests agains all transports.

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Do you guys have a preferred library to use as the sftp client? Should we use SSHD (https://mina.apache.org/sshd-project/index.html) or JSch(http://www.jcraft.com/jsch/)? SSHD seems to also provide a sftp server implementation which would be useful for fixtures.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Daz DeBoer-2



On Sun, Feb 23, 2014 at 3:12 PM, Marcin Erdmann <[hidden email]> wrote:



On Thu, Feb 20, 2014 at 8:33 PM, Adam Murdoch <[hidden email]> wrote:

On 19 Feb 2014, at 3:06 pm, Daz DeBoer <[hidden email]> wrote:

Hey Marcin
That's great. If you want to take a stab at a design spec for supporting an sftp repository transport, that would be a good start. This can go into a new document: /design-docs/repository-transports.md.

For the first story I think it should be sufficient to simply support a 'sftp://host[:port]' type URL, together with the existing user/password credentials. But we should probably start thinking about how we can support other authentication mechanisms as well (for all transports)

That would be good. I would love to make credentials a first-class concept and expose some nice command-line and tooling API mechanisms to provide and manage them.


We also need to start thinking about how we're going to provide adequate test coverage for different repository transports. Do we want to run all of our remote resolution tests against all remote transports, or choose a subset of tests that would be sufficient for demonstrating a particular transport behaves correctly? We can probably come up with a org.gradle.integtests.fixtures.AbstractMultiTestRunner subclass that can run a set of tests against a defined set of transports.

One thing that the current test coverage does is to verify exactly which network requests are being made when. This is really important from a performance point of view. I would not want to lose this coverage if we were to abstract some of the tests away from transport, and any new transports should have a similar kind of coverage. Perhaps we might change the test fixtures to abstract some of the verifications (there’s really only a handful of different interactions we need to check for).

Seems like a valid point. Looks like you're talking about specs that are using org.gradle.test.fixtures.maven.HttpResource.expect...() methods. We could abstract that to something like RemoteResource and use that to run all these tests agains all transports.

Yes. It seems like we could extract an API out of org.gradle.test.fixtures.server.http.HttpServer which would be implemented by org.gradle.test.fixtures.server.sftp.SFTPServer as well. Then the fixtures in org.gradle.test.fixtures.ivy and org.gradle.test.fixtures.maven could be adapted to deal in terms of this 'server expectation' API. This would allow us to run any tests that us  these fixtures against both server implementations.

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.
 

Do you guys have a preferred library to use as the sftp client? Should we use SSHD (https://mina.apache.org/sshd-project/index.html) or JSch(http://www.jcraft.com/jsch/)? SSHD seems to also provide a sftp server implementation which would be useful for fixtures.

I don't really have experience with either of these, but it looks like we're currently using both in our test fixture code (org.gradle.test.fixtures.server.sftp.SFTPServer) as well as providing jcraft for Ivy to use in it's SFtpResolver.

Thanks
Daz

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi



On Sun, Feb 23, 2014 at 10:33 PM, Daz DeBoer <[hidden email]> wrote:

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.

Would be great if you could draft it. You probably have a much better idea about the requirements than I do. 
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch
In reply to this post by erdi

On 24 Feb 2014, at 9:12 am, Marcin Erdmann <[hidden email]> wrote:




On Thu, Feb 20, 2014 at 8:33 PM, Adam Murdoch <[hidden email]> wrote:

On 19 Feb 2014, at 3:06 pm, Daz DeBoer <[hidden email]> wrote:

Hey Marcin
That's great. If you want to take a stab at a design spec for supporting an sftp repository transport, that would be a good start. This can go into a new document: /design-docs/repository-transports.md.

For the first story I think it should be sufficient to simply support a 'sftp://host[:port]' type URL, together with the existing user/password credentials. But we should probably start thinking about how we can support other authentication mechanisms as well (for all transports)

That would be good. I would love to make credentials a first-class concept and expose some nice command-line and tooling API mechanisms to provide and manage them.


We also need to start thinking about how we're going to provide adequate test coverage for different repository transports. Do we want to run all of our remote resolution tests against all remote transports, or choose a subset of tests that would be sufficient for demonstrating a particular transport behaves correctly? We can probably come up with a org.gradle.integtests.fixtures.AbstractMultiTestRunner subclass that can run a set of tests against a defined set of transports.

One thing that the current test coverage does is to verify exactly which network requests are being made when. This is really important from a performance point of view. I would not want to lose this coverage if we were to abstract some of the tests away from transport, and any new transports should have a similar kind of coverage. Perhaps we might change the test fixtures to abstract some of the verifications (there’s really only a handful of different interactions we need to check for).

Seems like a valid point. Looks like you're talking about specs that are using org.gradle.test.fixtures.maven.HttpResource.expect...() methods. We could abstract that to something like RemoteResource and use that to run all these tests agains all transports.

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Do you guys have a preferred library to use as the sftp client? Should we use SSHD (https://mina.apache.org/sshd-project/index.html) or JSch(http://www.jcraft.com/jsch/)? SSHD seems to also provide a sftp server implementation which would be useful for fixtures.

I’d slightly prefer we use apache sshd rather than jsch, but it’s not a strong opinion.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch
In reply to this post by erdi

On 24 Feb 2014, at 9:36 am, Marcin Erdmann <[hidden email]> wrote:




On Sun, Feb 23, 2014 at 10:33 PM, Daz DeBoer <[hidden email]> wrote:

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.

Would be great if you could draft it. You probably have a much better idea about the requirements than I do. 

If you’re keen to get started while we’re getting the spec together, you might do a quick spike with one or both of the clients to see what’s required to get them to talk to an ssh server, and if you have a preference between sshd or jsch. You could even start with an integration test that uses the sshd test fixture and drives the client(s) directly. We could then refactor this into the real implementation.

The sorts of things we’ll need to be able to do:

- Read the meta-data for a file, including whether it exists or not, plus the size and last modified time of the file.
- Read from a file.
- Write to a file.
- Create file.
- List the entries of a directory.

When things fail, it would be really nice to be able tell the difference between a failure where the thing did not exist, vs a failure where there is some permission problem, vs the server is not running, vs everything else.

I suspect error handling will be the deciding factor between the two clients.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi
Sounds like a plan.


On Sun, Feb 23, 2014 at 11:02 PM, Adam Murdoch <[hidden email]> wrote:

On 24 Feb 2014, at 9:36 am, Marcin Erdmann <[hidden email]> wrote:




On Sun, Feb 23, 2014 at 10:33 PM, Daz DeBoer <[hidden email]> wrote:

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.

Would be great if you could draft it. You probably have a much better idea about the requirements than I do. 

If you’re keen to get started while we’re getting the spec together, you might do a quick spike with one or both of the clients to see what’s required to get them to talk to an ssh server, and if you have a preference between sshd or jsch. You could even start with an integration test that uses the sshd test fixture and drives the client(s) directly. We could then refactor this into the real implementation.

The sorts of things we’ll need to be able to do:

- Read the meta-data for a file, including whether it exists or not, plus the size and last modified time of the file.
- Read from a file.
- Write to a file.
- Create file.
- List the entries of a directory.

When things fail, it would be really nice to be able tell the difference between a failure where the thing did not exist, vs a failure where there is some permission problem, vs the server is not running, vs everything else.

I suspect error handling will be the deciding factor between the two clients.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Daz DeBoer-2
I've pushed an initial cut of the spec: feel free to modify as you discover things that don't make sense.



On Sun, Feb 23, 2014 at 4:10 PM, Marcin Erdmann <[hidden email]> wrote:
Sounds like a plan.


On Sun, Feb 23, 2014 at 11:02 PM, Adam Murdoch <[hidden email]> wrote:

On 24 Feb 2014, at 9:36 am, Marcin Erdmann <[hidden email]> wrote:




On Sun, Feb 23, 2014 at 10:33 PM, Daz DeBoer <[hidden email]> wrote:

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.

Would be great if you could draft it. You probably have a much better idea about the requirements than I do. 

If you’re keen to get started while we’re getting the spec together, you might do a quick spike with one or both of the clients to see what’s required to get them to talk to an ssh server, and if you have a preference between sshd or jsch. You could even start with an integration test that uses the sshd test fixture and drives the client(s) directly. We could then refactor this into the real implementation.

The sorts of things we’ll need to be able to do:

- Read the meta-data for a file, including whether it exists or not, plus the size and last modified time of the file.
- Read from a file.
- Write to a file.
- Create file.
- List the entries of a directory.

When things fail, it would be really nice to be able tell the difference between a failure where the thing did not exist, vs a failure where there is some permission problem, vs the server is not running, vs everything else.

I suspect error handling will be the deciding factor between the two clients.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch

Looks good to me. I’ve added a few more things - we should reuse the transports and caching for remote build scripts, and add some basic credentials management as well (eg prompting at the command-line, tooling API integration)

On 24 Feb 2014, at 1:46 pm, Daz DeBoer <[hidden email]> wrote:

I've pushed an initial cut of the spec: feel free to modify as you discover things that don't make sense.



On Sun, Feb 23, 2014 at 4:10 PM, Marcin Erdmann <[hidden email]> wrote:
Sounds like a plan.


On Sun, Feb 23, 2014 at 11:02 PM, Adam Murdoch <[hidden email]> wrote:

On 24 Feb 2014, at 9:36 am, Marcin Erdmann <[hidden email]> wrote:




On Sun, Feb 23, 2014 at 10:33 PM, Daz DeBoer <[hidden email]> wrote:

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.

Would be great if you could draft it. You probably have a much better idea about the requirements than I do. 

If you’re keen to get started while we’re getting the spec together, you might do a quick spike with one or both of the clients to see what’s required to get them to talk to an ssh server, and if you have a preference between sshd or jsch. You could even start with an integration test that uses the sshd test fixture and drives the client(s) directly. We could then refactor this into the real implementation.

The sorts of things we’ll need to be able to do:

- Read the meta-data for a file, including whether it exists or not, plus the size and last modified time of the file.
- Read from a file.
- Write to a file.
- Create file.
- List the entries of a directory.

When things fail, it would be really nice to be able tell the difference between a failure where the thing did not exist, vs a failure where there is some permission problem, vs the server is not running, vs everything else.

I suspect error handling will be the deciding factor between the two clients.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com







--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi
I finally got my head around the contract of ExternalResourceAccessor and after doing some reverse engineering of HttpResourceAccessor and Ivy's SFTPResolver as well as skimming SFTP protocol specs the following are my findings and questions:

Error handling is far from perfect in both clients - they only return a generic error when you try to get file metadata for a file that doesn't exist but there seems to be a specific substatus for such situation called SSH_FX_NO_SUCH_FILE. I can see that this substatus is returned by SSHD's implementation of SFTP server when accessing files that don't exist even though the spec(http://tools.ietf.org/id/draft-ietf-secsh-filexfer-00.txt) cryptically describes it as "is returned when a reference is made to a file which should exist but doesn't" - this should but doesn't part baffles me a bit. Anyway I will assume that this substatus is returned whenever trying to stat a non-existing file.

I'm going to use SSHD's client as it seems to be better maintained and I like it more. I will be also able to easily change it's behavior when it comes to handling the aforementioned SSH_FX_NO_SUCH_FILE substatus.

We will probably want to throw a specific contextual exception when credentials are incorrect, right?

It feels like we want to have separate ssh sessions for getting metadata and getting the file - the fact that you request an ExternalResource doesn't mean that you will ever read it and thus close it, but you need to open a ssh session to get the metadata. So the idea here is to open a session, get metadata to see if the resource exists and close the session possibly passing the metadata to the created ExternalResource if the resource exists. Then, if resource contents are requested open a new session and close it when close() is called on the ExternalResource. Are we ok with such approach? I can't seem to find a way of reusing sessions without ending up with dangling ssh sessions in some situations.

Cheers,
Marcin


On Wed, Feb 26, 2014 at 10:00 PM, Adam Murdoch <[hidden email]> wrote:

Looks good to me. I’ve added a few more things - we should reuse the transports and caching for remote build scripts, and add some basic credentials management as well (eg prompting at the command-line, tooling API integration)

On 24 Feb 2014, at 1:46 pm, Daz DeBoer <[hidden email]> wrote:

I've pushed an initial cut of the spec: feel free to modify as you discover things that don't make sense.



On Sun, Feb 23, 2014 at 4:10 PM, Marcin Erdmann <[hidden email]> wrote:
Sounds like a plan.


On Sun, Feb 23, 2014 at 11:02 PM, Adam Murdoch <[hidden email]> wrote:

On 24 Feb 2014, at 9:36 am, Marcin Erdmann <[hidden email]> wrote:




On Sun, Feb 23, 2014 at 10:33 PM, Daz DeBoer <[hidden email]> wrote:

I didn't get much time to have a look at this stuff this weekend because I spent it contributing to Ratpack but I seem to have found test code that exercises transports so I should be able to get started this week. I will look at this code more and start drafting the design spec.

Excellent. I was planning on putting a first draft of the design spec up today; likely pretty basic to start with.

Would be great if you could draft it. You probably have a much better idea about the requirements than I do. 

If you’re keen to get started while we’re getting the spec together, you might do a quick spike with one or both of the clients to see what’s required to get them to talk to an ssh server, and if you have a preference between sshd or jsch. You could even start with an integration test that uses the sshd test fixture and drives the client(s) directly. We could then refactor this into the real implementation.

The sorts of things we’ll need to be able to do:

- Read the meta-data for a file, including whether it exists or not, plus the size and last modified time of the file.
- Read from a file.
- Write to a file.
- Create file.
- List the entries of a directory.

When things fail, it would be really nice to be able tell the difference between a failure where the thing did not exist, vs a failure where there is some permission problem, vs the server is not running, vs everything else.

I suspect error handling will be the deciding factor between the two clients.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com







--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch

On 6 Mar 2014, at 7:33 am, Marcin Erdmann <[hidden email]> wrote:

I finally got my head around the contract of ExternalResourceAccessor and after doing some reverse engineering of HttpResourceAccessor and Ivy's SFTPResolver as well as skimming SFTP protocol specs the following are my findings and questions:

Error handling is far from perfect in both clients - they only return a generic error when you try to get file metadata for a file that doesn't exist but there seems to be a specific substatus for such situation called SSH_FX_NO_SUCH_FILE. I can see that this substatus is returned by SSHD's implementation of SFTP server when accessing files that don't exist even though the spec(http://tools.ietf.org/id/draft-ietf-secsh-filexfer-00.txt) cryptically describes it as "is returned when a reference is made to a file which should exist but doesn't" - this should but doesn't part baffles me a bit. Anyway I will assume that this substatus is returned whenever trying to stat a non-existing file.

I'm going to use SSHD's client as it seems to be better maintained and I like it more. I will be also able to easily change it's behavior when it comes to handling the aforementioned SSH_FX_NO_SUCH_FILE substatus.

We will probably want to throw a specific contextual exception when credentials are incorrect, right?

At this stage, I think it’s sufficient that the error message gives the user some idea that the credentials are incorrect. Eventually, it would be nice to tell the difference, if possible.


It feels like we want to have separate ssh sessions for getting metadata and getting the file - the fact that you request an ExternalResource doesn't mean that you will ever read it and thus close it, but you need to open a ssh session to get the metadata. So the idea here is to open a session, get metadata to see if the resource exists and close the session possibly passing the metadata to the created ExternalResource if the resource exists. Then, if resource contents are requested open a new session and close it when close() is called on the ExternalResource. Are we ok with such approach?

Yes, for now. I think at some point we will rework the ExternalResourceAccessor interface so that it can better deal with the fact that for some transports (file, sftp), the meta-data for a resource and the content for a resource have to be fetched as separate operations. The interface at the moment assumes that it’s cheap-ish to get the meta-data at the same time the content is requested.

This way, the caller can decide whether it needs the meta-data or not, and invoke the appropriate operations - get meta-data, get content or get meta-data and content (if supported).


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi
I've been quiet for a while but made some progress on this. I have all resolving, some error and one uploading test passing from the first story described in the design doc. Yet, there is plenty to be done on error handling and making sure all resources are being released even in error scenarios - so the boring but important parts.

I have one question about error handling. There are three types of failures listed in the spec: invalid credentials, cannot connect to the server and everything else (called server exception). I think that we probably want to extract all permission related errors (when both resolving and uploading) as a separate error type so that the error message provides user an idea that the error is permission related as it feels that it might be a common error. What do you think?

Also, DefaultExternalResourceRepository requires an implementation of an ExternalResourceLister yet it is currently not exercised by any test - to be honest I haven't looked into how it's used and what kind of tests would be needed for it so any pointers would be appreciated. Do we want to add some tests to the initial story for that?


On Thu, Mar 6, 2014 at 3:56 AM, Adam Murdoch <[hidden email]> wrote:

On 6 Mar 2014, at 7:33 am, Marcin Erdmann <[hidden email]> wrote:

I finally got my head around the contract of ExternalResourceAccessor and after doing some reverse engineering of HttpResourceAccessor and Ivy's SFTPResolver as well as skimming SFTP protocol specs the following are my findings and questions:

Error handling is far from perfect in both clients - they only return a generic error when you try to get file metadata for a file that doesn't exist but there seems to be a specific substatus for such situation called SSH_FX_NO_SUCH_FILE. I can see that this substatus is returned by SSHD's implementation of SFTP server when accessing files that don't exist even though the spec(http://tools.ietf.org/id/draft-ietf-secsh-filexfer-00.txt) cryptically describes it as "is returned when a reference is made to a file which should exist but doesn't" - this should but doesn't part baffles me a bit. Anyway I will assume that this substatus is returned whenever trying to stat a non-existing file.

I'm going to use SSHD's client as it seems to be better maintained and I like it more. I will be also able to easily change it's behavior when it comes to handling the aforementioned SSH_FX_NO_SUCH_FILE substatus.

We will probably want to throw a specific contextual exception when credentials are incorrect, right?

At this stage, I think it’s sufficient that the error message gives the user some idea that the credentials are incorrect. Eventually, it would be nice to tell the difference, if possible.


It feels like we want to have separate ssh sessions for getting metadata and getting the file - the fact that you request an ExternalResource doesn't mean that you will ever read it and thus close it, but you need to open a ssh session to get the metadata. So the idea here is to open a session, get metadata to see if the resource exists and close the session possibly passing the metadata to the created ExternalResource if the resource exists. Then, if resource contents are requested open a new session and close it when close() is called on the ExternalResource. Are we ok with such approach?

Yes, for now. I think at some point we will rework the ExternalResourceAccessor interface so that it can better deal with the fact that for some transports (file, sftp), the meta-data for a resource and the content for a resource have to be fetched as separate operations. The interface at the moment assumes that it’s cheap-ish to get the meta-data at the same time the content is requested.

This way, the caller can decide whether it needs the meta-data or not, and invoke the appropriate operations - get meta-data, get content or get meta-data and content (if supported).


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch

On 15 Mar 2014, at 6:31 am, Marcin Erdmann <[hidden email]> wrote:

I've been quiet for a while but made some progress on this. I have all resolving, some error and one uploading test passing from the first story described in the design doc.

Great.

Yet, there is plenty to be done on error handling and making sure all resources are being released even in error scenarios - so the boring but important parts.

I have one question about error handling. There are three types of failures listed in the spec: invalid credentials, cannot connect to the server and everything else (called server exception). I think that we probably want to extract all permission related errors (when both resolving and uploading) as a separate error type so that the error message provides user an idea that the error is permission related as it feels that it might be a common error. What do you think?

The key is to make the error message informative at this stage, which we don’t necessarily need a new type for. If we formalise ‘not authorised’ problems in the resource APIs then we would also need to update the file and http backed resource implementations to. So, I’d leave this as a later refactoring.


Also, DefaultExternalResourceRepository requires an implementation of an ExternalResourceLister yet it is currently not exercised by any test - to be honest I haven't looked into how it's used and what kind of tests would be needed for it so any pointers would be appreciated. Do we want to add some tests to the initial story for that?

It’s used whenever there’s a dynamic version in a dependency declaration. So, if you’ve got one of these in one of the tests somewhere, then we're good.



On Thu, Mar 6, 2014 at 3:56 AM, Adam Murdoch <[hidden email]> wrote:

On 6 Mar 2014, at 7:33 am, Marcin Erdmann <[hidden email]> wrote:

I finally got my head around the contract of ExternalResourceAccessor and after doing some reverse engineering of HttpResourceAccessor and Ivy's SFTPResolver as well as skimming SFTP protocol specs the following are my findings and questions:

Error handling is far from perfect in both clients - they only return a generic error when you try to get file metadata for a file that doesn't exist but there seems to be a specific substatus for such situation called SSH_FX_NO_SUCH_FILE. I can see that this substatus is returned by SSHD's implementation of SFTP server when accessing files that don't exist even though the spec(http://tools.ietf.org/id/draft-ietf-secsh-filexfer-00.txt) cryptically describes it as "is returned when a reference is made to a file which should exist but doesn't" - this should but doesn't part baffles me a bit. Anyway I will assume that this substatus is returned whenever trying to stat a non-existing file.

I'm going to use SSHD's client as it seems to be better maintained and I like it more. I will be also able to easily change it's behavior when it comes to handling the aforementioned SSH_FX_NO_SUCH_FILE substatus.

We will probably want to throw a specific contextual exception when credentials are incorrect, right?

At this stage, I think it’s sufficient that the error message gives the user some idea that the credentials are incorrect. Eventually, it would be nice to tell the difference, if possible.


It feels like we want to have separate ssh sessions for getting metadata and getting the file - the fact that you request an ExternalResource doesn't mean that you will ever read it and thus close it, but you need to open a ssh session to get the metadata. So the idea here is to open a session, get metadata to see if the resource exists and close the session possibly passing the metadata to the created ExternalResource if the resource exists. Then, if resource contents are requested open a new session and close it when close() is called on the ExternalResource. Are we ok with such approach?

Yes, for now. I think at some point we will rework the ExternalResourceAccessor interface so that it can better deal with the fact that for some transports (file, sftp), the meta-data for a resource and the content for a resource have to be fetched as separate operations. The interface at the moment assumes that it’s cheap-ish to get the meta-data at the same time the content is requested.

This way, the caller can decide whether it needs the meta-data or not, and invoke the appropriate operations - get meta-data, get content or get meta-data and content (if supported).


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com






--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi



On Fri, Mar 14, 2014 at 7:56 PM, Adam Murdoch <[hidden email]> wrote:

Yet, there is plenty to be done on error handling and making sure all resources are being released even in error scenarios - so the boring but important parts.

I have one question about error handling. There are three types of failures listed in the spec: invalid credentials, cannot connect to the server and everything else (called server exception). I think that we probably want to extract all permission related errors (when both resolving and uploading) as a separate error type so that the error message provides user an idea that the error is permission related as it feels that it might be a common error. What do you think?

The key is to make the error message informative at this stage, which we don’t necessarily need a new type for. If we formalise ‘not authorised’ problems in the resource APIs then we would also need to update the file and http backed resource implementations to. So, I’d leave this as a later refactoring.

What I meant here by a new type is not a new exception class but to differentiate permission errors from generic errors when it comes to what message is being shown when such an error occurs. At the moment I'm throwing a new runtime exception of type SftpException with a message saying what has happened (invalid credentials, connection error) and where (host and port part of the url). I was asking if we should check if we have permissions to perform a given operation (file download or upload, directory creation, etc) and if not throw a SftpException with a message saying that permission was denied to do it.
 


Also, DefaultExternalResourceRepository requires an implementation of an ExternalResourceLister yet it is currently not exercised by any test - to be honest I haven't looked into how it's used and what kind of tests would be needed for it so any pointers would be appreciated. Do we want to add some tests to the initial story for that?

It’s used whenever there’s a dynamic version in a dependency declaration. So, if you’ve got one of these in one of the tests somewhere, then we're good.

I don't have a test like that yet so I will add some. Thanks for pointing me in the right direction.
 



On Thu, Mar 6, 2014 at 3:56 AM, Adam Murdoch <[hidden email]> wrote:

On 6 Mar 2014, at 7:33 am, Marcin Erdmann <[hidden email]> wrote:

I finally got my head around the contract of ExternalResourceAccessor and after doing some reverse engineering of HttpResourceAccessor and Ivy's SFTPResolver as well as skimming SFTP protocol specs the following are my findings and questions:

Error handling is far from perfect in both clients - they only return a generic error when you try to get file metadata for a file that doesn't exist but there seems to be a specific substatus for such situation called SSH_FX_NO_SUCH_FILE. I can see that this substatus is returned by SSHD's implementation of SFTP server when accessing files that don't exist even though the spec(http://tools.ietf.org/id/draft-ietf-secsh-filexfer-00.txt) cryptically describes it as "is returned when a reference is made to a file which should exist but doesn't" - this should but doesn't part baffles me a bit. Anyway I will assume that this substatus is returned whenever trying to stat a non-existing file.

I'm going to use SSHD's client as it seems to be better maintained and I like it more. I will be also able to easily change it's behavior when it comes to handling the aforementioned SSH_FX_NO_SUCH_FILE substatus.

We will probably want to throw a specific contextual exception when credentials are incorrect, right?

At this stage, I think it’s sufficient that the error message gives the user some idea that the credentials are incorrect. Eventually, it would be nice to tell the difference, if possible.


It feels like we want to have separate ssh sessions for getting metadata and getting the file - the fact that you request an ExternalResource doesn't mean that you will ever read it and thus close it, but you need to open a ssh session to get the metadata. So the idea here is to open a session, get metadata to see if the resource exists and close the session possibly passing the metadata to the created ExternalResource if the resource exists. Then, if resource contents are requested open a new session and close it when close() is called on the ExternalResource. Are we ok with such approach?

Yes, for now. I think at some point we will rework the ExternalResourceAccessor interface so that it can better deal with the fact that for some transports (file, sftp), the meta-data for a resource and the content for a resource have to be fetched as separate operations. The interface at the moment assumes that it’s cheap-ish to get the meta-data at the same time the content is requested.

This way, the caller can decide whether it needs the meta-data or not, and invoke the appropriate operations - get meta-data, get content or get meta-data and content (if supported).


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com






--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

erdi
I have another question. One of test cases in the spec is Publish via 'sftp' to ivy repository with multiple ivyPattern and artifactPattern configured - what do we want to verify here? I've noticed that when publishing to such a repository the first ivy pattern and artifact pattern are used. Should the test case assert that this happens?


On Fri, Mar 14, 2014 at 8:22 PM, Marcin Erdmann <[hidden email]> wrote:



On Fri, Mar 14, 2014 at 7:56 PM, Adam Murdoch <[hidden email]> wrote:

Yet, there is plenty to be done on error handling and making sure all resources are being released even in error scenarios - so the boring but important parts.

I have one question about error handling. There are three types of failures listed in the spec: invalid credentials, cannot connect to the server and everything else (called server exception). I think that we probably want to extract all permission related errors (when both resolving and uploading) as a separate error type so that the error message provides user an idea that the error is permission related as it feels that it might be a common error. What do you think?

The key is to make the error message informative at this stage, which we don’t necessarily need a new type for. If we formalise ‘not authorised’ problems in the resource APIs then we would also need to update the file and http backed resource implementations to. So, I’d leave this as a later refactoring.

What I meant here by a new type is not a new exception class but to differentiate permission errors from generic errors when it comes to what message is being shown when such an error occurs. At the moment I'm throwing a new runtime exception of type SftpException with a message saying what has happened (invalid credentials, connection error) and where (host and port part of the url). I was asking if we should check if we have permissions to perform a given operation (file download or upload, directory creation, etc) and if not throw a SftpException with a message saying that permission was denied to do it.
 


Also, DefaultExternalResourceRepository requires an implementation of an ExternalResourceLister yet it is currently not exercised by any test - to be honest I haven't looked into how it's used and what kind of tests would be needed for it so any pointers would be appreciated. Do we want to add some tests to the initial story for that?

It’s used whenever there’s a dynamic version in a dependency declaration. So, if you’ve got one of these in one of the tests somewhere, then we're good.

I don't have a test like that yet so I will add some. Thanks for pointing me in the right direction.
 



On Thu, Mar 6, 2014 at 3:56 AM, Adam Murdoch <[hidden email]> wrote:

On 6 Mar 2014, at 7:33 am, Marcin Erdmann <[hidden email]> wrote:

I finally got my head around the contract of ExternalResourceAccessor and after doing some reverse engineering of HttpResourceAccessor and Ivy's SFTPResolver as well as skimming SFTP protocol specs the following are my findings and questions:

Error handling is far from perfect in both clients - they only return a generic error when you try to get file metadata for a file that doesn't exist but there seems to be a specific substatus for such situation called SSH_FX_NO_SUCH_FILE. I can see that this substatus is returned by SSHD's implementation of SFTP server when accessing files that don't exist even though the spec(http://tools.ietf.org/id/draft-ietf-secsh-filexfer-00.txt) cryptically describes it as "is returned when a reference is made to a file which should exist but doesn't" - this should but doesn't part baffles me a bit. Anyway I will assume that this substatus is returned whenever trying to stat a non-existing file.

I'm going to use SSHD's client as it seems to be better maintained and I like it more. I will be also able to easily change it's behavior when it comes to handling the aforementioned SSH_FX_NO_SUCH_FILE substatus.

We will probably want to throw a specific contextual exception when credentials are incorrect, right?

At this stage, I think it’s sufficient that the error message gives the user some idea that the credentials are incorrect. Eventually, it would be nice to tell the difference, if possible.


It feels like we want to have separate ssh sessions for getting metadata and getting the file - the fact that you request an ExternalResource doesn't mean that you will ever read it and thus close it, but you need to open a ssh session to get the metadata. So the idea here is to open a session, get metadata to see if the resource exists and close the session possibly passing the metadata to the created ExternalResource if the resource exists. Then, if resource contents are requested open a new session and close it when close() is called on the ExternalResource. Are we ok with such approach?

Yes, for now. I think at some point we will rework the ExternalResourceAccessor interface so that it can better deal with the fact that for some transports (file, sftp), the meta-data for a resource and the content for a resource have to be fetched as separate operations. The interface at the moment assumes that it’s cheap-ish to get the meta-data at the same time the content is requested.

This way, the caller can decide whether it needs the meta-data or not, and invoke the appropriate operations - get meta-data, get content or get meta-data and content (if supported).


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com






--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Contribution request: add support for SFtp and SSH for publishing and resolving

Adam Murdoch
In reply to this post by erdi

On 15 Mar 2014, at 7:22 am, Marcin Erdmann <[hidden email]> wrote:




On Fri, Mar 14, 2014 at 7:56 PM, Adam Murdoch <[hidden email]> wrote:

Yet, there is plenty to be done on error handling and making sure all resources are being released even in error scenarios - so the boring but important parts.

I have one question about error handling. There are three types of failures listed in the spec: invalid credentials, cannot connect to the server and everything else (called server exception). I think that we probably want to extract all permission related errors (when both resolving and uploading) as a separate error type so that the error message provides user an idea that the error is permission related as it feels that it might be a common error. What do you think?

The key is to make the error message informative at this stage, which we don’t necessarily need a new type for. If we formalise ‘not authorised’ problems in the resource APIs then we would also need to update the file and http backed resource implementations to. So, I’d leave this as a later refactoring.

What I meant here by a new type is not a new exception class but to differentiate permission errors from generic errors when it comes to what message is being shown when such an error occurs. At the moment I'm throwing a new runtime exception of type SftpException with a message saying what has happened (invalid credentials, connection error) and where (host and port part of the url). I was asking if we should check if we have permissions to perform a given operation (file download or upload, directory creation, etc) and if not throw a SftpException with a message saying that permission was denied to do it.

It depends how you’re proposing to implement it. If it’s something that we can figure out from the response to the operation, then that makes sense. If it's something we figure out by doing another request prior to the operation, then that’s less interesting, as it’s a potential performance problem, plus the answer can change in the window between the 2 requests. One alternative to this would be to do the permission check only if the original operation fails for some opaque reason, to provide better diagnostics.


--
Adam Murdoch
Gradle Co-founder
http://www.gradle.org
VP of Engineering, Gradleware Inc. - Gradle Training, Support, Consulting
http://www.gradleware.com



12
Loading...