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 <marcin.erdm...@proxerd.pl>wrote: > > > > On Fri, Mar 14, 2014 at 7:56 PM, Adam Murdoch <adam.murd...@gradleware.com > > 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 <adam.murd...@gradleware.com >> > wrote: >> >>> >>> On 6 Mar 2014, at 7:33 am, Marcin Erdmann <marcin.erdm...@proxerd.pl> >>> 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 >> >> >> >> >
