https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422018-05-02T14:24:54ZArvadosArvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=622842018-05-02T14:24:54ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Related to</strong> <i><a class="issue tracker-2 status-3 priority-4 priority-default closed" href="/issues/11184">Feature #11184</a>: [Keep] Support multiple storage classes</i> added</li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=622882018-05-02T14:26:44ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> set to <i>To Be Groomed</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=623052018-05-02T16:18:03ZTom Cleggtom@curii.com
<ul><li><strong>Subject</strong> changed from <i>arvados-cwl-runner support storage tiers</i> to <i>[API] [arvados-cwl-runner] Save workflow outputs to desired storage classes</i></li><li><strong>Description</strong> updated (<a title="View differences" href="/journals/62305/diff?detail_id=59344">diff</a>)</li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=623112018-05-02T16:19:22ZTom Cleggtom@curii.com
<ul><li><strong>Related to</strong> deleted (<i><a class="issue tracker-2 status-3 priority-4 priority-default closed" href="/issues/11184">Feature #11184</a>: [Keep] Support multiple storage classes</i>)</li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=623122018-05-02T16:19:30ZTom Cleggtom@curii.com
<ul><li><strong>Blocks</strong> <i><a class="issue tracker-2 status-3 priority-4 priority-default closed" href="/issues/11184">Feature #11184</a>: [Keep] Support multiple storage classes</i> added</li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=623342018-05-02T17:08:08ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/62334/diff?detail_id=59372">diff</a>)</li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=623372018-05-02T17:10:19ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>To Be Groomed</i> to <i>Arvados Future Sprints</i></li><li><strong>Story points</strong> set to <i>1.0</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=629102018-05-23T14:57:24ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2018-06-06 Sprint</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=629242018-05-23T15:08:44ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Fuad Muhic</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=631102018-06-04T17:54:45ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Reviewing <a class="changeset" title="Add storage classes supper to arvados cwl runner Arvados-DCO-1.1-Signed-off-by: Fuad Muhic <fmuh..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/95c01d22571043d7b6405f928538204cf930d453">95c01d22571043d7b6405f928538204cf930d453</a></p>
<ul>
<li>File <code>sdk/cwl/arvados_cwl/__init__.py</code>
<ul>
<li>Line 604: I think the default storage classes value could be set on the <code>parser.add_argument()</code> call</li>
<li>Line 606: On 13430 one of the requirements was to accept just one storage class for the moment, it that also desirable on this story?</li>
<li>Line 727: Typo on help message: 'wortkflow'</li>
</ul>
</li>
<li>I think some tests are needed for cases when the required storage classes is not the default value, and to check that it errors out when passed multiple storage classes.</li>
</ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=631732018-06-06T12:27:08ZFuad Muhicfmuhic@capeannenterprises.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=631752018-06-06T13:10:17ZFuad Muhicfmuhic@capeannenterprises.com
<ul></ul><p>I added check and tests for multiple storage classes. I think it would be nice to have a test that checks if output collection<br />contains storage_classes_desired field set to proper value defined by --storage-classes argument. However I'm really not sure how to write that test, I tried but I guess I'm not experienced enough with cwl-runner.</p> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=631902018-06-06T14:48:48ZFuad Muhicfmuhic@capeannenterprises.com
<ul><li><strong>Target version</strong> changed from <i>2018-06-06 Sprint</i> to <i>2018-06-20 Sprint</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=632402018-06-06T17:36:18ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Reviewing <a class="changeset" title="Add check for multiple storage classes in arvados-cwl-runner Arvados-DCO-1.1-Signed-off-by: Fuad..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/b01b480adb45c3bbfcfab13e343e08c16854dedc">b01b480adb45c3bbfcfab13e343e08c16854dedc</a></p>
<ul>
<li>File <code>sdk/cwl/tests/test_submit.py</code>
* Line 338: <code>capture_stdout</code> is not being used after the <code>main()</code> call so I suppose it can avoided or read from it to assure that the correct error message is being printed?</li>
<li>File <code>sdk/cwl/arvados_cwl/__init__.py</code>
* Line 789: On error I think the way to go is to log it and return 1 (as in lines 804 & 824)</li>
<li>I think a test that assures an output collection has correct correct <code>storage_classes_desired</code> field is not necessary as the Python SDK Collection’s test already proves that. OTOH I think having a test that confirms that default & non-default <code>—storage-classes</code> values are passed correctly to <code>make_output_collection()</code> will be very useful. I think this can be done using a similar approach as your <code>test_error_when_multiple_storage_classes_specified</code> test and mocking <code>make_output_collection</code> to check how it was called.</li>
<li>I realized that the storage classes argument is being used only when <code>—submit</code> is not added. When it is used, a-c-r submits a container that runs another a-c-r with all necessary params (see test at <code>test_submit.py</code> line 591, for example), so we’ll need a similar test that assures that the correct@ —storage-classes@ param is being passed along. I’m not sure where this needs to be added, please talk to Peter if you don’t find it.</li>
<li>Also, remember to merge master into this branch so Jenkins test don't fail because of <code>ciso8601</code></li>
</ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=635372018-06-18T10:02:33ZFuad Muhicfmuhic@capeannenterprises.com
<ul></ul><p>tests: <a class="external" href="https://ci.curoverse.com/job/developer-run-tests/759/">https://ci.curoverse.com/job/developer-run-tests/759/</a> (with merged master)</p>
<ul>
<li>Added test which check that storage_classes parameters is propagated correctly to make_output_collection</li>
<li>In --submit mode, storage_classes are passes to submitted container. (I opted to pass --storage-classes parameter only if user specified if in the first place. If user don't specify --storage-classes, submitted container will not have --storage-classes=default. Everything still works correctly and we don't have to add 100+ lines to test_submit.py). I added tests for this.</li>
</ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=635492018-06-18T13:41:03ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>This LGTM. Thanks!</p> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=635542018-06-18T14:50:49ZFuad Muhicfmuhic@capeannenterprises.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset <a class="changeset" title="Merge branch '13429-cwl-runner-storage-classes-support' closes #13429 Arvados-DCO-1.1-Signed-of..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/3fbabc1b5236b6667f453a2949849d96f6e683df">arvados|3fbabc1b5236b6667f453a2949849d96f6e683df</a>.</p> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=635562018-06-18T14:52:23ZFuad Muhicfmuhic@capeannenterprises.com
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Closed</i></li></ul> Arvados - Idea #13429: [API] [arvados-cwl-runner] Save workflow outputs to desired storage classeshttps://dev.arvados.org/issues/13429?journal_id=647302018-07-23T18:41:47ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Release</strong> set to <i>13</i></li></ul>