Bug #17016

"Remove file" deletes whole collection?

Added by Peter Amstutz 6 months ago. Updated 3 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
Start date:
12/01/2020
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Description

  1. Create a collection with arv-put. It consists of two files in a subdirectory.
  2. Go to workbench2
  3. Select the first file
  4. Click on the ... menu on the right
  5. Choose "Remove"
  6. Hit ok
  7. Instead of just one file, all the files are gone.

This behaves inconsistently. I've also tried to delete a single item in a folder and it deleted the whole folder (but not the whole collection).


Subtasks

Task #17025: Review 17016-delete-single-file-deletes-whole-collectionResolvedLucas Di Pentima


Related issues

Related to Arvados - Bug #15771: Deleting a selection of files failsResolved01/12/2021

Associated revisions

Revision 7437e0b4
Added by Daniel Kutyła 3 months ago

Merge branch '17016-delete-single-file-deletes-whole-collection'
closes #17016

Arvados-DCO-1.1-Signed-off-by: Daniel Kutyła <>

History

#1 Updated by Peter Amstutz 6 months ago

  • Subject changed from Remove file deletes whole project? to "Remove file" deletes whole collection?

#2 Updated by Peter Amstutz 6 months ago

  • Description updated (diff)

#3 Updated by Peter Amstutz 6 months ago

  • Related to Bug #15771: Deleting a selection of files fails added

#4 Updated by Peter Amstutz 6 months ago

  • Assigned To set to Daniel Kutyła

#5 Updated by Peter Amstutz 6 months ago

  • Target version changed from 2020-11-04 Sprint to 2020-11-18

#6 Updated by Peter Amstutz 5 months ago

  • Description updated (diff)

#7 Updated by Peter Amstutz 5 months ago

  • Target version changed from 2020-11-18 to 2020-12-02 Sprint

#8 Updated by Daniel Kutyła 5 months ago

New version first commit: https://dev.arvados.org/projects/arvados-workbench-2/repository/revisions/36d0bf2c656c5ff79c94216a8bfbed015bc84f7e
Test run: https://ci.arvados.org/job/developer-tests-workbench2/203/

Fixed duplicated file names, added sorting in order to avoid of folder being removed before the files within them

#9 Updated by Daniel Kutyła 5 months ago

  • Status changed from New to In Progress

#10 Updated by Peter Amstutz 5 months ago

  • Target version changed from 2020-12-02 Sprint to 2020-12-16 Sprint

#11 Updated by Daniel Kutyła 5 months ago

Code mentioned above should also fix #15771 as this bug is closely related

#12 Updated by Lucas Di Pentima 5 months ago

Reviewing arvados-workbench2|36d0bf2 - branch 17016-delete-single-file-deletes-whole-collection

  • The remove feature seems to work correctly now, thanks! Would you add an integration test with a case similar to what this ticket describes? (remove file in subdir) You could make one of those by creating a collection with a synthetic manifest_text like other cypress test already do, so that you don't need to upload real files.
  • From reading the code and doing some testing on my own, the received paths are sorted by length descending, I suppose this is to avoid getting errors when trying to remove files that were inside subdirectories already removed. This looks ok at first but when selecting several parts of a very deep and/or populated file tree, this would make the wb2 issue too many webdav requests by individually removing many files that could be just removed in one sweep because the parent directory is already scheduled for removal. Could we make the opposite? Like, starting removing the shorter paths and once they’re gone, avoid sending delete requests to dirs/files inside the already removed ones?

#13 Updated by Peter Amstutz 4 months ago

  • Target version changed from 2020-12-16 Sprint to 2021-01-06 Sprint

#15 Updated by Peter Amstutz 3 months ago

  • Target version changed from 2021-01-06 Sprint to 2021-01-20 Sprint

#16 Updated by Lucas Di Pentima 3 months ago

Danny: Not sure if this is ready for review, but I took a look at it anyways.

  • The integration tests look good. One suggestion, though: You can modify the manifest_text on the second test to directly create a collection with files inside a subdirectory by replacing . with ./subdir on the file entries, that way you could simplify a lot the test because you won't have to make it move the file. You can read more about the manifest text format here: https://doc.arvados.org/v2.1/architecture/manifest-format.html
  • My second bulletpoint at note-12 is still pending to be addressed.

#18 Updated by Lucas Di Pentima 3 months ago

This works great, thanks!

As one last request: Could you write a unit test that test this last optimization? I think it would be really useful to have automated warranties that this keeps working as expected in the future.

#20 Updated by Lucas Di Pentima 3 months ago

This LGTM, please merge. Thanks!

#21 Updated by Daniel Kutyła 3 months ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF