Delete sequence files from file storage via sequence run page #1468

ksierks · 2023-02-14T20:35:09Z

Description of changes

Added functionality to delete sequence files from file storage (local, Azure, and AWS) when removing a sequence run.

Related issue

#1125

Checklist

Things for the developer to confirm they've done before the PR should be accepted:

CHANGELOG.md (and UPGRADING.md if necessary) updated with information for new change.
Tests added (or description of how to test) for any new features.
~~[ ] User documentation updated for UI or technical changes.~~

…leStorageUtility

github-actions · 2023-02-14T20:36:01Z

...orefacility/bioinformatics/irida/repositories/filesystem/IridaFileStorageAwsUtilityImpl.java

@@ -257,7 +268,7 @@ public void appendToFile(Path target, SequenceFile file) throws IOException {
 		try (FileChannel out = FileChannel.open(target, StandardOpenOption.CREATE, StandardOpenOption.APPEND,
 				StandardOpenOption.WRITE)) {
 			try (FileChannel in = new FileInputStream(iridaTemporaryFile.getFile().toFile()).getChannel()) {
-				for (long p = 0, l = in.size(); p < l;) {
+				for (long p = 0, l = in.size(); p < l; ) {


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyForIteratorPadCheck> _{reported by reviewdog 🐶}
';' is followed by whitespace.

github-actions · 2023-02-14T20:36:02Z

...efacility/bioinformatics/irida/repositories/filesystem/IridaFileStorageLocalUtilityImpl.java

@@ -151,7 +164,7 @@ public void appendToFile(Path target, SequenceFile file) throws IOException {
 		try (FileChannel out = FileChannel.open(target, StandardOpenOption.CREATE, StandardOpenOption.APPEND,
 				StandardOpenOption.WRITE)) {
 			try (FileChannel in = FileChannel.open(file.getFile(), StandardOpenOption.READ)) {
-				for (long p = 0, l = in.size(); p < l;) {
+				for (long p = 0, l = in.size(); p < l; ) {


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyForIteratorPadCheck> _{reported by reviewdog 🐶}
';' is followed by whitespace.

github-actions · 2023-02-14T20:36:02Z

...efacility/bioinformatics/irida/repositories/filesystem/IridaFileStorageAzureUtilityImpl.java

@@ -234,7 +243,7 @@ public void appendToFile(Path target, SequenceFile file) throws IOException {
 		try (FileChannel out = FileChannel.open(target, StandardOpenOption.CREATE, StandardOpenOption.APPEND,
 				StandardOpenOption.WRITE)) {
 			try (FileChannel in = new FileInputStream(iridaTemporaryFile.getFile().toFile()).getChannel()) {
-				for (long p = 0, l = in.size(); p < l;) {
+				for (long p = 0, l = in.size(); p < l; ) {


🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.whitespace.EmptyForIteratorPadCheck> _{reported by reviewdog 🐶}
';' is followed by whitespace.

JeffreyThiessen

2 notes:

When removing a run via the admin sequencing run screen, only the unzipped files are deleted, and the gzipped files remain. This can be seen by running ls /tmp/irida/sequence-files/*/* Is this intended behaviour?
A bug occurs when removing a run via the admin sequencing run screen, when sequencing files have manually been removed.

Here 1 file has been deleted

Then the run gets removed via the admin sequencing run screen, then go back to the project to see this

I found it easiest to test these cases by removing all the files in /tmp/irida/sequence-files/ and /tmp/irida/output-files/ , and start irida with a new db each time

ksierks · 2023-02-21T20:48:36Z

2 notes:

When removing a run via the admin sequencing run screen, only the unzipped files are deleted, and the gzipped files remain. This can be seen by running ls /tmp/irida/sequence-files/*/* Is this intended behaviour?

A bug occurs when removing a run via the admin sequencing run screen, when sequencing files have manually been removed.

Here 1 file has been deleted

Then the run gets removed via the admin sequencing run screen, then go back to the project to see this

I found it easiest to test these cases by removing all the files in /tmp/irida/sequence-files/ and /tmp/irida/output-files/ , and start irida with a new db each time

Thanks for the review, Jeff.

Please see 655f175.
I'm not introducing this bug in this PR. I believe it exists in development. When the file is removed from the sample, so is the sequencing object. When the sequencing object is removed it breaks the link to the sequencing run. Will address this in a future PR.

deepsidhu85

The code is looking good just one thing below for now. I still need to test it out

...efacility/bioinformatics/irida/repositories/filesystem/IridaFileStorageAzureUtilityImpl.java

deepsidhu85

This is looking good..seems to be working as expected. Just a few small things below

...ility/bioinformatics/irida/repositories/filesystem/FilesystemSupplementedRepositoryImpl.java

.../corefacility/bioinformatics/irida/ria/integration/cloud/IridaFileStorageAwsUtilityTest.java

...orefacility/bioinformatics/irida/ria/integration/cloud/IridaFileStorageAzureUtilityTest.java

JeffreyThiessen

Please see 655f175.
I'm not introducing this bug in this PR. I believe it exists in development. When the file is removed from the sample, so is the sequencing object. When the sequencing object is removed it breaks the link to the sequencing run. Will address this in a future PR.

I confirmed it's happening in development too. It only leaves the sample on the page, the underlying files are all removed correctly.

Code looks great too.

starting to delete sequence files from filesystem through the IridaFi…

55ba275

…leStorageUtility

github-actions bot reviewed Feb 14, 2023

View reviewed changes

ksierks added 4 commits February 14, 2023 16:02

fixing comments

78ca65d

Merge branch 'development' into filesystem-cleanup-sequence-runs

7f8a495

adding a test

489fdb8

fixing linting errors

f46ffbe

ksierks changed the title ~~Delete sequence files from file system - Sequence Runs~~ Delete sequence files from file system via sequence run page Feb 16, 2023

ksierks added 5 commits February 16, 2023 13:33

adding Azure deleteFile() functionality

df3e75b

adding AWS deleteFile() functionality

4930a3a

Merge branch 'development' into filesystem-cleanup-sequence-runs

6a92894

fixing linting errors

dd88a80

updating changelog

0c4bda4

ksierks marked this pull request as ready for review February 20, 2023 15:13

ksierks requested review from ericenns, apetkau and JeffreyThiessen and removed request for apetkau and JeffreyThiessen February 20, 2023 15:14

ksierks changed the title ~~Delete sequence files from file system via sequence run page~~ Delete sequence files from file storage via sequence run page Feb 20, 2023

JeffreyThiessen requested changes Feb 20, 2023

View reviewed changes

ksierks added 2 commits February 24, 2023 19:36

adding delete folder functionality

655f175

fixing test

8ec2e9a

ksierks requested a review from deepsidhu85 February 27, 2023 16:10

ksierks added 2 commits February 27, 2023 10:35

adding missed exception handling

283d479

adding deleteFolder tests

02d9bf0

deepsidhu85 requested changes Feb 27, 2023

View reviewed changes

...efacility/bioinformatics/irida/repositories/filesystem/IridaFileStorageAzureUtilityImpl.java Show resolved Hide resolved

ksierks added 2 commits February 27, 2023 15:57

commenting out as it's causing me grief

7f37a1a

adding missed exception handling

23cb0f3

ksierks requested a review from deepsidhu85 February 28, 2023 14:29

ksierks requested a review from JeffreyThiessen February 28, 2023 14:29

Merge branch 'development' into filesystem-cleanup-sequence-runs

814bb6f

deepsidhu85 requested changes Mar 3, 2023

View reviewed changes

ksierks added 3 commits March 3, 2023 11:06

fixing folder tests

8ff3e0d

moving if statement

53f1df3

fixing folder tests

1506132

deepsidhu85 approved these changes Mar 3, 2023

View reviewed changes

JeffreyThiessen approved these changes Mar 3, 2023

View reviewed changes

ericenns removed their request for review March 9, 2023 20:20

deepsidhu85 merged commit 7c54a65 into development Mar 9, 2023

ericenns deleted the filesystem-cleanup-sequence-runs branch April 14, 2023 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete sequence files from file storage via sequence run page #1468

Delete sequence files from file storage via sequence run page #1468

ksierks commented Feb 14, 2023 •

edited

Loading

github-actions bot Feb 14, 2023

github-actions bot Feb 14, 2023

github-actions bot Feb 14, 2023

JeffreyThiessen left a comment •

edited

Loading

ksierks commented Feb 21, 2023 •

edited

Loading

deepsidhu85 left a comment

deepsidhu85 left a comment

JeffreyThiessen left a comment

Delete sequence files from file storage via sequence run page #1468

Delete sequence files from file storage via sequence run page #1468

Conversation

ksierks commented Feb 14, 2023 • edited Loading

Description of changes

Related issue

Checklist

github-actions bot Feb 14, 2023

Choose a reason for hiding this comment

github-actions bot Feb 14, 2023

Choose a reason for hiding this comment

github-actions bot Feb 14, 2023

Choose a reason for hiding this comment

JeffreyThiessen left a comment • edited Loading

Choose a reason for hiding this comment

ksierks commented Feb 21, 2023 • edited Loading

deepsidhu85 left a comment

Choose a reason for hiding this comment

deepsidhu85 left a comment

Choose a reason for hiding this comment

JeffreyThiessen left a comment

Choose a reason for hiding this comment

ksierks commented Feb 14, 2023 •

edited

Loading

JeffreyThiessen left a comment •

edited

Loading

ksierks commented Feb 21, 2023 •

edited

Loading