WIP: Next steps #1488

antgonza · 2015-10-06T19:39:53Z

This is based on #1481 so that should be merged first.

Here I'm fixing the last standing points in the joy list, so far:

Fix Store EBI submission files with the preprocessed data #1479
In the template documentation changing that sample description is a required field for Qiita deploy to a requirement for EBI, as this is required for submissions.
Removing 2 cfg variables that are not needed anymore with the new EBI system. Today we got a notification that the old system will be out in 2 months.
Changes so tests pass in the main branch.

…iita into next-joy-list

…navas/qiita into next-joy-list

…iita into next-joy-list

josenavas · 2015-10-08T23:20:36Z

qiita_pet/support_files/doc/source/tutorials/prepare-templates.rst

@@ -98,6 +96,8 @@ These are the columns required for successfully submit your data to EBI:
 +-------------------------------------+-------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
 | ``library_construction_protocol``   | free text                                 | Brief description or reference to the protocol that was used for preparing this amplicon library starting from DNA, usually this includes what genomic region was targeted such as *16S*, *ITS*, *18S*, etc. |
 +-------------------------------------+-------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+| ``description``                     | free text                                 | Description of the sample.                                                                                                                                                                                   |


Why has this been moved from the sample template to the prep template? IIRC, there was a problem if we had duplicated columns between the sample template and the prep template, and the description was required already in the sample template. Also, I think the description applies to the "biological sample" (i.e. sample template) rather than to the preparation sample.

My error, this was meant to be in the required fields for EBI in the sample template, not the prep template, will change.

ElDeveloper · 2015-10-09T04:46:12Z

I'm having a hard time figuring out what are the specifics that need review in this PR, maybe once #1490 is merged, it will be easier.

…nto next-joy-list

antgonza · 2015-10-09T14:17:58Z

This is ready for final review. Note that this solves all the open issues in the joy list: #1451. Once this is merged, we just need to fix conflicts between master and the ebi-improvements branch ...

mortonjt · 2015-10-09T16:12:26Z

qiita_ware/test/test_commands.py

+        return ppd
+
+    def test_submit_EBI_step_2_failure(self):
+        pass


Do we still want these tests? Or should they be dropped?

ElDeveloper · 2015-10-09T18:00:59Z

👍 done with my review!

josenavas · 2015-10-09T18:17:47Z

qiita_ware/commands.py

+        for cmd in ebi_submission.generate_send_sequences_cmd():
+            cmd_pieces = shsplit(cmd)
+            try:
+                call(cmd_pieces, stdout=open(ebi_submission.ascp_reply, 'a'))


Is it worth using moi's moi.job.system_call function? It does the stdout and stderr parsing for you and it will reduce code in here.

ElDeveloper · 2015-10-09T18:20:46Z

👍 on my end, @josenavas, please merge once your review comments are addressed 👟

josenavas · 2015-10-09T18:23:09Z

qiita_ware/commands.py

        LogEntry.create('Runtime',
                        ("Submitting XMLs for pre_processed_id: "
                         "%d" % preprocessed_data_id))
        try:
-            # place holder for moi call see #1477
-            xmls_cmds_moi = xmls_cmds
+            call(xmls_cmds, stdout=open(ebi_submission.curl_reply, 'w'))


I think it may be useful to store the stderr too, in case that something unexpected happens (not only in EBI end)

josenavas · 2015-10-09T20:37:27Z

qiita_ware/ebi.py


            demux = [path for _, path, ftype in ppd.get_filepaths()
                     if ftype == 'preprocessed_demux'][0]

            demux_samples = set()
            with open_file(demux) as demux_fh:
+                if not isinstance(demux_fh, File):
+                    error_msg = ("There was an error parsing the demux file: "


This error message can be improved, as the error is that file is not a demux file.

josenavas · 2015-10-10T00:22:17Z

qiita_ware/commands.py

        else:
            LogEntry.create('Runtime',
                            ('Submission of sequences of pre_processed_id: '
                             '%d completed successfully' %
                             preprocessed_data_id))
+        finally:


If system_call raises, xml_content is not defined, and stderr doesn't contain the value from such call (it will have the value from the above call to system_call in the loop.

josenavas · 2015-10-10T00:25:19Z

2 comments and tests are still failing. It looks like now you're installing ascp, so why is that test failing?

antgonza · 2015-10-10T15:10:45Z

@josenavas this is ready. After your comments I realized that we weren't doing the same for all possible failures in commands.py, which I fixed. Also, fixed some warnings that were left over cause missing instrument_model.

josenavas · 2015-10-10T16:36:18Z

Thanks @antgonza
awesome work!

WIP: Next steps

antgonza added 10 commits October 6, 2015 08:44

fix qiita-spots#1479

2215406

description is required for EBI

c0c3caa

removing unnecessary EBI cfg vars

42b73ed

changing conf file and fixing setup_acsp.sh

72cac32

catching to ebi-improvements

52f6496

Merge branch 'ebi-improvements-pet' of https://github.com/josenavas/q…

29c945e

…iita into next-joy-list

initial changes test_commands.py

c45be2e

Merge branch 'ebi-improvements-run-setter' of https://github.com/jose…

b2be7e0

…navas/qiita into next-joy-list

test ready

1928106

Merge branch 'ebi-improvements-pet' of https://github.com/josenavas/q…

1c7ef8e

…iita into next-joy-list

antgonza mentioned this pull request Oct 8, 2015

[WIP] EBI improvements #1451

Merged

14 tasks

josenavas reviewed Oct 8, 2015
View reviewed changes

Merge branch 'ebi-improvements' of https://github.com/biocore/qiita i…

fe1070d

…nto next-joy-list

antgonza mentioned this pull request Oct 9, 2015

Add ascp to travis to test EBI #406

Closed

fixing last issues

6417aa6

mortonjt reviewed Oct 9, 2015
View reviewed changes

addressing @ElDeveloper and @mortonjt comments

abc5fe8

josenavas reviewed Oct 9, 2015
View reviewed changes

addressing @josenavas comments

13f80b1

josenavas reviewed Oct 9, 2015
View reviewed changes

fixing error

0586d8b

josenavas reviewed Oct 10, 2015
View reviewed changes

antgonza added 6 commits October 10, 2015 05:16

fixing build errors

30723c2

testing if

f57a364

fixing leftover if

a7f7dfe

invert if

9377a3a

fixing ignore

d2eae77

fixing error

a368f41

josenavas added a commit that referenced this pull request Oct 10, 2015

Merge pull request #1488 from antgonza/next-joy-list

0a47d89

WIP: Next steps

josenavas merged commit 0a47d89 into qiita-spots:ebi-improvements Oct 10, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Next steps #1488

WIP: Next steps #1488

antgonza commented Oct 6, 2015

josenavas Oct 8, 2015

antgonza Oct 8, 2015

ElDeveloper commented Oct 9, 2015

antgonza commented Oct 9, 2015

mortonjt Oct 9, 2015

ElDeveloper commented Oct 9, 2015

josenavas Oct 9, 2015

ElDeveloper commented Oct 9, 2015

josenavas Oct 9, 2015

josenavas Oct 9, 2015

josenavas Oct 10, 2015

josenavas commented Oct 10, 2015

antgonza commented Oct 10, 2015

josenavas commented Oct 10, 2015

WIP: Next steps #1488

WIP: Next steps #1488

Conversation

antgonza commented Oct 6, 2015

josenavas Oct 8, 2015

Choose a reason for hiding this comment

antgonza Oct 8, 2015

Choose a reason for hiding this comment

ElDeveloper commented Oct 9, 2015

antgonza commented Oct 9, 2015

mortonjt Oct 9, 2015

Choose a reason for hiding this comment

ElDeveloper commented Oct 9, 2015

josenavas Oct 9, 2015

Choose a reason for hiding this comment

ElDeveloper commented Oct 9, 2015

josenavas Oct 9, 2015

Choose a reason for hiding this comment

josenavas Oct 9, 2015

Choose a reason for hiding this comment

josenavas Oct 10, 2015

Choose a reason for hiding this comment

josenavas commented Oct 10, 2015

antgonza commented Oct 10, 2015

josenavas commented Oct 10, 2015