genouest / biomaj-download

Download microservice for BioMAJ
GNU Affero General Public License v3.0
1 stars 7 forks source link

Speed up IRODDownload #24

Closed duboism closed 4 years ago

duboism commented 4 years ago

This PR introduces a new hook method _network_configuration which is called before network methods (list and download) to perform configuration (this generalizes CurlDownload._basic_curl_configuration).

This greatly speeds up the download of many files for IRODSDownloader (mainly because we don't cleanup and recreate the session after each download) and a bit for CurlDownloader (because less reconfiguration of the curl object).

osallou commented 4 years ago

Is it ready for merge?

duboism commented 4 years ago

I think it is (unit tests are green and I have made some functional tests). Maybe we should just check that microservice mode is not broken.

osallou commented 4 years ago

Renaming will not impact microservices. Service or not, you get a handler and call list or download. Internal logic afterward does not matter .

Only constraint as we previously discussed is to not depend on bank info or property file directly (everything goes through message), which was already the case.

Le mer. 15 janv. 2020 à 22:09, duboism notifications@github.com a écrit :

I think it is (unit tests are green and I have made some functional tests). Maybe we should just check that microservice mode is not broken.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/genouest/biomaj-download/pull/24?email_source=notifications&email_token=AAH2AYRP6K72E6E4MRQCEK3Q553Q5A5CNFSM4KHHAMHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJB2ACQ#issuecomment-574857226, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2AYRTEZ7FH67MBE7OAYDQ553Q5ANCNFSM4KHHAMHA .

duboism commented 4 years ago

OK, just wanted to be sure. The PR can be merged.