https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422017-09-14T20:20:41ZArvadosArvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=550222017-09-14T20:20:41ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2017-09-27 Sprint</i></li></ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=550292017-09-14T20:40:08ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li></ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=554042017-09-27T15:22:10ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> changed from <i>2017-09-27 Sprint</i> to <i>2017-10-11 Sprint</i></li></ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=557282017-10-04T12:26:29ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>12125-workbench-project-trash @ <a class="changeset" title="12125: Expand test case. Tests trashing project and checking that it shows up on the trash page...." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/71da9f42b396e6ae8d7ef83b1855d5bb407c2a17">71da9f42b396e6ae8d7ef83b1855d5bb407c2a17</a></p>
<p>Adds "Trashed projects" tab to trash page.</p>
<p>Also adds check to 404 page if an item is in the trash, and offers to untrash it.</p>
<p>Adds tests.</p>
<p><a class="external" href="https://ci.curoverse.com/job/developer-run-tests/474/">https://ci.curoverse.com/job/developer-run-tests/474/</a></p> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=557432017-10-04T17:12:13ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><ul>
<li>On the 404 page, do you think that adding the date when the trashed item is going to be deleted is something that could be useful to the user?</li>
<li>When accessing a collection that’s inside a trashed project, the 404 doesn’t notify the user this fact, and visiting the Trash page doesn’t show the collection, even searching it by uuid. Maybe the easier path to allow the user find those kind of lost collections is to inform that it’s parent project is trashed?</li>
<li>The above also happens with projects inside a trashed project, it doesn’t appear on Trash and the 404 page just says that’s not found.</li>
<li>If the previous points are valid, maybe it would be convenient to write tests about them.</li>
</ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=557842017-10-06T13:44:49ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Lucas Di Pentima wrote:</p>
<blockquote>
<ul>
<li>On the 404 page, do you think that adding the date when the trashed item is going to be deleted is something that could be useful to the user?</li>
</ul>
</blockquote>
<p>Sure, can't hurt.</p>
<blockquote>
<ul>
<li>When accessing a collection that’s inside a trashed project, the 404 doesn’t notify the user this fact, and visiting the Trash page doesn’t show the collection, even searching it by uuid. Maybe the easier path to allow the user find those kind of lost collections is to inform that it’s parent project is trashed?</li>
</ul>
</blockquote>
<p>Yes, the 404 page could follow the ownership links to find the trashed project and offer to untrash it.</p>
<blockquote>
<ul>
<li>The above also happens with projects inside a trashed project, it doesn’t appear on Trash and the 404 page just says that’s not found.</li>
</ul>
</blockquote>
<p>We have an <code>include_trashed</code> flag which would allow us to search for items regardless of whether they are trashed or not. However, this creates a couple of challenges with the current API:</p>
<ul>
<li>We can't efficiently determine if an item is inside a trashed project </li>
<li>We can't efficiently determine which trashed project contains an item</li>
</ul>
<p>Doing this for a single items (on the 404 page) isn't so bad, but doing it across sets of items is much more expensive.</p>
<p>I think the ideal API would be to add a "inside_trashed_project_uuid" field that is returned on all objects when include_trashed=true, however that will require reopening <a class="issue tracker-6 status-3 priority-4 priority-default closed parent" title="Idea: [API] Allow projects to be deleted (ie placed in the trash can) (Resolved)" href="https://dev.arvados.org/issues/12032">#12032</a> first.</p>
<blockquote>
<ul>
<li>If the previous points are valid, maybe it would be convenient to write tests about them.</li>
</ul>
</blockquote>
<p>They are valid, I'm just not sure whether to address them or not.</p> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=557942017-10-06T19:53:38ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Strategy</p>
<ul>
<li>When listing/searching trashed collections
<ul>
<li>use include_trashed</li>
<li>collect owner uuids and query that without include_trashed</li>
<li>trashed collections will either
<ul>
<li>have trashed_at set (trashed directly)</li>
<li>have a owner uuid that isn't in the list (trashed indirectly)</li>
</ul>
</li>
<li>may need to iterate a few times so we have enough items to show</li>
</ul></li>
</ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=558992017-10-11T18:24:56ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li><li><strong>Target version</strong> changed from <i>2017-10-11 Sprint</i> to <i>2017-10-25 Sprint</i></li></ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=561812017-10-23T12:41:55ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>12125-workbench-project-trash @ <a class="changeset" title="12125: Update trash tests Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz@veritasgenetics..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/6e68c94524cfb19fc0b51591eab7e4a55e485164">6e68c94524cfb19fc0b51591eab7e4a55e485164</a></p>
<p>When visiting the trash page, initially only is_trashed=true items are shown. However, user can search for trashed collections and subprojects that are contained within a trashed project. There is a note on the page informing the user about this behavior.</p>
<p>The rational for this behavior is:</p>
<ol>
<li>Limits the visual clutter of the initial listing to only projects and collections that were explicitly trashed.</li>
<li>It requires two queries to fetch items within a trashed project, one to get all items, a second to get non-trashed items (which are subtracted from the "all" list to get just the trashed items). This is relatively expensive so limiting it to searches ensures it is only done for queries where the result set is expected to be small.</li>
</ol>
<p>Rows returned within a trashed project as a result of a search behave slightly differently from rows where is_trashed=true. To avoid expensive lookups during page rendering, the trash_at and delete_at times are not displayed. In addition, the selection check box is not rendered, because the item can't be untrashed directly, but requires untrashing the parent.</p>
<p>The 404 page now informs the user when an item is part of a trashed project, and offers to untrash the parent project. If a project is under multiple levels of trash (for example, a trashed collection in a trashed project) the user will click through multiple untrash pages until the item is fully untrashed.</p>
<p>The recycle button to untrash a single item on the trash page now directs to the 404 page for consistency in the flow for untrashing a single item whether it is within a trashed project or trashed directly.</p> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=561892017-10-23T17:23:28ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>The new behavior looks good to me. I ran workbench tests locally, got some failures:</p>
<pre>
5) Error:
JobsTest#test_view_log_via_keep-web_redirect:
Capybara::ExpectationNotMet: expected to find text "log message 1" in "workbench:test Dashboard Projects Home A Project Trash (No description provided) Copy to project... Move job... Status Log Details Provenance Advanced Summary Download the full log Log Select all Select none Sort by time Sort by node Sort by task Show all tasks Only successful tasks Only failed tasks Show crunch diagnostics Show task dispatch Show task diagnostics Show compute usage Timestamp Node Slot Log type Task Message"
test/integration/jobs_test.rb:80:in `block in <class:JobsTest>'
test/test_helper.rb:303:in `run'
6) Error:
AnonymousAccessTest#test_view_file:
Capybara::ExpectationNotMet: expected to find text "7med30yd5b23ekjqyfswrpg15bf0glmwnkoto2imzdw0jxsyw1ghs8oecdhn5k295quvdo87ekmrrxmql49ff06phl9oob07jk1" in "workbench:test Browse public projects Log in Public Projects Unrestricted public data (none) (none) Collection UUID: Content address: Content size: 1 file totalling 99 bytes No source information available. Welcome to Arvados You are accessing public data. Files Tags Provenance graph Used by Advanced 99 bytes Hello world.txt"
test/integration/anonymous_access_test.rb:130:in `block in <class:AnonymousAccessTest>'
test/test_helper.rb:303:in `run'
7) Failure:
CollectionsTest#test_can_download_an_entire_collection_with_a_reader_token [/home/lucas/arvados_local/apps/workbench/test/integration/collections_test.rb:91]:
download page did provide strictly file links.
Expected: ["foo"]
Actual: []
</pre> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=561962017-10-23T20:08:42ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Passes on jenkins:</p>
<p><a class="external" href="https://ci.curoverse.com/job/developer-run-tests/488/">https://ci.curoverse.com/job/developer-run-tests/488/</a></p> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=562042017-10-24T14:02:53ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Reviewed branch <code>12125-fuse-project-trash</code> - <a class="changeset" title="12125: Add test deleting project via API updates FUSE. Arvados-DCO-1.1-Signed-off-by: Peter Amst..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/4d2dfa766a8a78b4f3f303d1d8d8dfe7488a85af">4d2dfa766</a></p>
<p>LGTM. Just a tiny observation:</p>
<ul>
<li>File services/fuse/tests/test_mount.py - Line 768: I think the use of <code>sorted()</code> is unnecessary, correct?</li>
</ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=562142017-10-24T19:09:17ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul> Arvados - Idea #12125: Client support for deleting projectshttps://dev.arvados.org/issues/12125?journal_id=562152017-10-24T19:10:31ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Lucas Di Pentima wrote:</p>
<blockquote>
<p>Reviewed branch <code>12125-fuse-project-trash</code> - <a class="changeset" title="12125: Add test deleting project via API updates FUSE. Arvados-DCO-1.1-Signed-off-by: Peter Amst..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/4d2dfa766a8a78b4f3f303d1d8d8dfe7488a85af">4d2dfa766</a></p>
<p>LGTM. Just a tiny observation:</p>
<ul>
<li>File services/fuse/tests/test_mount.py - Line 768: I think the use of <code>sorted()</code> is unnecessary, correct?</li>
</ul>
</blockquote>
<p>Yes, that's a copy and paste from another bit of code that expected more than one element. I choose to merge it as-is, but thanks for pointing it out.</p>