EBI step 2

[antgonza]

In this PR I'm removing all kwargs from methods, generating step 2 of the pipeline (generate per sample files) and adding tests for it.

[josenavas]

Does this even work? I though that the insdc_status was a controlled vocab, except for the failed: ... status.

[antgonza]

Not sure, but at this point I'm just setting this section as it should work and then cleaning as needed. Note that the issue is that there is no tests for this file ... I'm gonna need to add it in a future iteration. Already added it to the joy list: #1451

[josenavas]

Thanks, I'm unsure if this function even needs to exist... We probably only want to use the parallel wrapper directly and this function should disappear. However, we will see as the code evolves...

[josenavas]

@antgonza I'm a bit confused about this. IIRC, we agreed on using the ParallelWrapper right? Are you just cleaning up the submit_EBI to just have the 5 steps now and then change it later to make usage of the parallel wrapper? I just want to make sure that we are in the same page.

[antgonza]

5 steps: Yup, same page.

[josenavas]

Thanks @antgonza

[josenavas]

Just to make sure that I understand what is going on here. Is this else covering the case in which the per sample FASTQ's have been already generated? If so, can you add a comment to make it clear?

[antgonza]

Done as a Note.

[josenavas]

Few comments.

[josenavas]

I don't think this is needed.

Actually, I just noticed that this test file is unsafe!!! Can you put the class decorator @qiita_test_checker() to the test class, so we avoid to run the test if this a production environment? That decorator will automatically drop the schema after each test, so you don't need to delete the processed data, only the files that it generates.

[antgonza]

Good point, made the change but note that I divided the test in readonly and write/read cause without that decorator the test took 0.246s vs. with all the tests with the decorator it took 31.447s so with that split it takes 1.194s.

[josenavas]

Thanks @antgonza 👍
Splitting the tests between read-only and read-write is the way to go to speed up our test suite. I've only done it for the metadata template classes but we have an issue (#1029) to keep track of that. We may need to extend that list to all our tests

[antgonza]

Can this get another review? @ElDeveloper @squirrelo @adamrp

[ElDeveloper]

Preferably, this should be using path.join instead of string formatting.

[adamrp]

👍

[adamrp]

A few comments, but I think the code looks good. We should think carefully about what we want to do for UPDATEs, because I think they are the trickiest part here.

• For updates of metadata where none of the samples have been added or removed, I think it is relatively straightforward just to change the action that is passed.
• For all other updates (where samples are added or removed, or sequence files need to be updated), I believe we cannot actually do the update automatically. However, if we store the proper information in the database (i.e., the EBI aliases for everything that we submit), then we can automatically send an email requesting changes.

[antgonza]

@adamrp, thanks. I have added your comment about update to the joy list: #1451

[antgonza]

Ping??

[ElDeveloper]

Instead of just saving the exception message, I would save the whole traceback (for debugging purposes), seems to be something like this:

>>> from traceback import format_exc
>>> try:
...    1/0
... except:
...    x = format_exc()
... 
>>> print x
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: integer division or modulo by zero

[ElDeveloper]

The whole thing looks good to me, just one more comment and this should be ready for merge.

[adamrp]

👍