Closed duboism closed 4 years ago
Is it ready for merge? If yes, could you add modification info in changes file?
Hello,
I think the PR is ready for merge. I have updated CHANGES.txt
. Shall I increment the version in setup.py
?
@osallou Thanks for merging. I think that genouest/biomaj#120 can be merged too.
Based on the merged code the examples in #29 are:
>>> sftpd = CurlDownload(curl_protocol="sftp", host="test.rebex.net", rootdir="/toto")
>>> sftpd.set_credentials("demo:password")
>>> sftpd.list()
Error while listing sftp://test.rebex.net/toto - (78, 'Could not open remote file for reading: SFTP server: Path not found.')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/mdubois/Code/BioMAJ/biomaj-download/biomaj_download/download/curl.py", line 432, in list
raise e
File "/home/mdubois/Code/BioMAJ/biomaj-download/biomaj_download/download/curl.py", line 423, in list
self.crl.perform()
pycurl.error: (78, 'Could not open remote file for reading: SFTP server: Path not found.')
And:
>>> sftpd = CurlDownload(curl_protocol="sftp", host="test.rebex.net", rootdir="/")
>>> sftpd.set_credentials("demo:badpassword")
>>> sftpd.list()
Error while listing sftp://test.rebex.net/ - (67, 'Authentication failure')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/mdubois/Code/BioMAJ/biomaj-download/biomaj_download/download/curl.py", line 432, in list
raise e
File "/home/mdubois/Code/BioMAJ/biomaj-download/biomaj_download/download/curl.py", line 423, in list
self.crl.perform()
pycurl.error: (67, 'Authentication failure')
This PR addresses issue #29. This is mainly raising errors when something goes wrong on
list
.There are 2 changes that could impact other aspects :
FTPS(S)
servers,cURL
issues aQUIT
command; thenRESPONSE_CODE
is221
instead of226
. 6d809f2 implements a list of accepted codes (instead of a single one). Note that forFTP(S)
,cURL
usually raises an exception so checkingRESPONSE_CODE
is not really needed. However, it doesn't raise an exception for404
errors when usingHTTP
(the option CURLOPT_FAILONERROR could help but it's not perfect). So using a list ofRESPONSE_CODE
is simpler.RSYNCDownload
no longerchidr
in constructor (see b68bb74). This was a check (as passing a local, non-existent directory would then raise an exception) but I was not sure if this was compatible in particular with microservices (what happen if we can't construct the downloader ? could changing the current directory affect other process, downloaders, etc. ?). According to our tests, this doesn't affect where files are downloaded.Functional tests are done in genouest/biomaj#120.
Note that the tests use assertLogs which requires python >= 3.4 so we needed to drop support for python 2 before (see afefe2d).