Fix formatting and add ascp password env variable

qiime / qiime_web_app

QIIME, EMP and American Gut Web Application

www.microbio.me/qiime

9 stars 10 forks source link

Fix formatting and add ascp password env variable #638

Closed ElDeveloper closed 10 years ago

ElDeveloper commented 10 years ago

There were a few problems with the new configuration that we got after contacting EBI, in short:

Passwords had to be updated, so we've centralized the retrieval of the credentials to be only in this file for the subprocesses that need them.
There was a missin " in the formatting of a string to generate the CURL commands.

Please note that once this branch is merged into master and deployed, we will have to modify the profile file for the system to be in concordance with this change set.

adamrp commented 10 years ago

Looks good to me! I think we should test without those legacy whitespaces to see if everything works and, if it does, we should remove them

ElDeveloper commented 10 years ago

Ok, that sounds reasonable. I'm going to assume they do not matter ...

ElDeveloper commented 10 years ago

@teravest @adamrp Do we want to keep holding off on this one? I don't remember why were we holding off though :shipit:

teravest commented 10 years ago

This looks good to me what about the legacy whitespaces? how did that test go?

ElDeveloper commented 10 years ago

I don't think we need the spaces and it should work without them but I left them there because that was in the code before. Will remove them if you guys think I should.

teravest commented 10 years ago

If it works without them we should remove them.

On Fri, Mar 7, 2014 at 1:37 PM, Yoshiki Vázquez Baeza < notifications@github.com> wrote:

I don't think we need the spaces and it should work without them but I left them there because that was in the code before. Will remove them if you guys think I should.

Reply to this email directly or view it on GitHubhttps://github.com/qiime/qiime_web_app/pull/638#issuecomment-37065076 .

adamrp commented 10 years ago

agree with @teravest

On Fri, Mar 7, 2014 at 1:40 PM, teravest notifications@github.com wrote:

If it works without them we should remove them.

On Fri, Mar 7, 2014 at 1:37 PM, Yoshiki Vázquez Baeza < notifications@github.com> wrote:

I don't think we need the spaces and it should work without them but I left them there because that was in the code before. Will remove them if you guys think I should.

Reply to this email directly or view it on GitHub< https://github.com/qiime/qiime_web_app/pull/638#issuecomment-37065076> .

Reply to this email directly or view it on GitHubhttps://github.com/qiime/qiime_web_app/pull/638#issuecomment-37065307 .

teravest commented 10 years ago

@ElDeveloper did you test this without the spaces?

ElDeveloper commented 10 years ago

Sorry, I completely forgot about this pull request, all should be fixed now.