ctubbsii commented on a change in pull request #2309:
URL: https://github.com/apache/accumulo/pull/2309#discussion_r726363305
##########
File path:
core/src/main/java/org/apache/accumulo/core/metadata/schema/ExternalCompactionMetadata.java
##########
@@ -54,6 +55,9 @@ public ExternalCompactionMetadata(Set<StoredTabletFile>
jobFiles, Set<StoredTabl
TabletFile compactTmpName, String compactorId, CompactionKind kind,
short priority,
CompactionExecutorId ceid, boolean propagateDeletes, boolean
initiallySelectedAll,
Long compactionId) {
+ if (!propagateDeletes && (kind == CompactionKind.SELECTOR || kind ==
CompactionKind.USER)) {
+ Preconditions.checkArgument(initiallySelectedAll);
+ }
Review comment:
Rather than split the overall condition, part of it being included in
the `if` clause, and part of it being passed to `checkArgument`, I would avoid
using Preconditions. Normally that's used as shorthand to avoid an explicit
`if` statement, but here, it isn't shortening anything. It's not even being
used to offer a descriptive message to the `IllegalArgumentException`. Instead,
consider something like:
```suggestion
if (!propagateDeletes && !initiallySelectedAll && (kind ==
CompactionKind.SELECTOR || kind == CompactionKind.USER)) {
throw new IllegalArgumentException("CompactionKind." + kind + " must
propagate deletes unless all files are initially selected");
}
```
If you really prefer the Preconditions, you could do the following (with a
static import, to make it shorter):
```suggestion
checkArgument(propagateDeletes || initiallySelectedAll || (kind !=
CompactionKind.SELECTOR && kind != CompactionKind.USER), "CompactionKind.%s
must propagate deletes unless all files are initially selected", kind);
```
(I have not attempted to format either of these suggestions)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
