You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/09/02 14:50:38 UTC
[GitHub] [commons-io] jvz opened a new pull request #72: [IO-596] Add DeleteFiles utility class
jvz opened a new pull request #72:
URL: https://github.com/apache/commons-io/pull/72
This class provides configurable strategies to delete files and directories.
Adapted from the work done in https://github.com/jenkinsci/jenkins/pull/3812.
Signed-off-by: Matt Sicker <bo...@gmail.com>
@reviewbybees @garydgregory @jeffret-b
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory edited a comment on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Hi All,
For the basic operations, please use version 2.7's `IOExceptionList` and `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
WRT:
- `maxRetries`
- `waitBetweenRetriesMillis`
- `backoffMultiplier`
- `retryOverridingFileAttributes`
- `overrideAllAttributes`
Perhaps these attributes could encapsulated in a new `CopyStrategy implements DeleteOption, CopyOption` class (where deleting is a kind of copying [to the bit bucket]) which could then be used by both _delete_ and _copy_ methods in `PathUtils`?
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-686463378
@jvz
Nice idea Matt :-) let's discuss on the ML where a retry API belongs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-687795186
On Wed, Sep 2, 2020 at 10:57 AM Matt Sicker <no...@github.com>
wrote:
> Breaking up into execution strategies sounds like a good idea! Such a
> strategy could likely adopt a similar API to retry libraries like retry4j.
>
I am cutting 2.8.0-RC2 now, so let's think about this for 2.9.0.
Gary
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-io/pull/72#issuecomment-685792005>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJB6NYP6MQVO34A72S3TSTSDZMNXANCNFSM4GMIUKMA>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-691327858
Another item: values like `waitBetweenRetriesMillis` are error prone in my experience. Over at HttpComponents, we have classes called TimeValue and Timeout that wrap a `long` and a `TimeUnit`, and I'd like to bring that in to Apache Commons, someplace...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-946928160
Hi @jvz
May you check PathUtils delete methods and related classes in git master and base further enhancements on that? I'll probably get a new release candidate out in a week or two. So this could be for this or the next release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-691327858
Another item: values like `waitBetweenRetriesMillis` are error prone in my experience. Over at HttpComponents, we have classes called TimeValue and Timeout that wrap a `long` and a `TimeUnit`, and I'd like to bring that in to Apache Commons, someplace...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory edited a comment on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Hi All,
For the basic operations, please use version 2.7's `IOExceptionList` and `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
WRT:
- maxRetries;
- waitBetweenRetriesMillis;
- backoffMultiplier;
- retryOverridingFileAttributes;
- overrideAllAttributes;
Perhaps these attributes could encapsulated in a new `CopyStrategy implements DeleteOption` class (where deleting is a kind of copying [to the bit bucket]) which could then be used by both _delete_ and _copy_ methods in `PathUtils`?
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory edited a comment on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Hi All,
For the basic operations, please use version 2.7's `IOExceptionList` and `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
WRT:
- maxRetries;
- waitBetweenRetriesMillis;
- backoffMultiplier;
- retryOverridingFileAttributes;
- overrideAllAttributes;
Perhaps these attributes could encapsulated in a new `CopyStrategy implements DeleteOption, CopyOption` class (where deleting is a kind of copying [to the bit bucket]) which could then be used by both _delete_ and _copy_ methods in `PathUtils`?
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory edited a comment on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Closing this PR, use version 2.7's `IOExceptionList` and `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
WRT:
- maxRetries;
- waitBetweenRetriesMillis;
- backoffMultiplier;
- retryOverridingFileAttributes;
- overrideAllAttributes;
Perhaps these attributes could encapsulated in a new `CopyStrategy` class (where deleting is a kind of copying [to the bit bucket]) which could then be used by both _delete_ and _copy_ methods in `PathUtils`?
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory closed pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory closed pull request #72:
URL: https://github.com/apache/commons-io/pull/72
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory edited a comment on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Closing this PR, use version 2.7's `IOExceptionList` and `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-691327858
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] jvz commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685792005
Breaking up into execution strategies sounds like a good idea! Such a strategy could likely adopt a similar API to retry libraries like retry4j.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-674342883
Some of this PR is likely obsolete due to the newer PathUtils delete methods. Please review these and perhaps propose enhancements?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-691327858
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on a change in pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #72:
URL: https://github.com/apache/commons-io/pull/72#discussion_r470932614
##########
File path: src/main/java/org/apache/commons/io/CompositeIOException.java
##########
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.io;
+
+import java.io.IOException;
+import java.util.List;
+
+/**
+ * Combines one or more IOExceptions into a single IOException.
+ */
+public class CompositeIOException extends IOException {
+ private static final long serialVersionUID = 1L;
+
+ private final IOException[] causes;
+
+ public CompositeIOException(final String message, final List<IOException> causes) {
+ super(message);
+ this.causes = causes.toArray(new IOException[0]);
+ }
+
+ public CompositeIOException(final String message, final IOException... causes) {
+ super(message);
+ this.causes = causes.clone();
+ }
+
+ public IOException[] getCauses() {
+ return causes.clone();
+ }
+}
Review comment:
Note that we have `IOExceptionList`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-914290060
Some of this is superseded by the new(ish) code in PathUtils.delete methods and associated utilities. If we want a retry (count/duration delay) feature, let's build it on top of the newer code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory commented on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Closing this PR, use version 2.7's `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-io] garydgregory edited a comment on pull request #72: [IO-596] Add DeleteFiles utility class
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #72:
URL: https://github.com/apache/commons-io/pull/72#issuecomment-685786776
Hi All,
For the basic operations, please use version 2.7's `IOExceptionList` and `PathUtils` methods:
- `delete(Path)`
- `delete(Path, DeleteOption...)`
- `deleteDirectory(Path)`
- `deleteDirectory(Path, DeleteOption...)`
- `deleteFile(Path)`
- `deleteFile(Path, DeleteOption...)`
WRT:
- maxRetries;
- waitBetweenRetriesMillis;
- backoffMultiplier;
- retryOverridingFileAttributes;
- overrideAllAttributes;
Perhaps these attributes could encapsulated in a new `CopyStrategy` class (where deleting is a kind of copying [to the bit bucket]) which could then be used by both _delete_ and _copy_ methods in `PathUtils`?
WDYT?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org