vikrambohra commented on code in PR #3725:
URL: https://github.com/apache/gobblin/pull/3725#discussion_r1278137375
##########
gobblin-completeness/src/main/java/org/apache/gobblin/completeness/verifier/KafkaAuditCountVerifier.java:
##########
@@ -217,7 +217,7 @@ private static void validateTierCounts(String datasetName,
long beginInMillis, l
throw new IOException(String.format("Reference tier %s audit count
cannot be retrieved for dataset %s between %s and %s", refTier, datasetName,
beginInMillis, endInMillis));
}
long refCount = countsByTier.get(refTier);
- if(refCount <= 0) {
+ if (refCount < 0) {
throw new IOException(String.format("Reference tier %s count cannot be
less than or equal to zero", refTier));
Review Comment:
Change exception log line to match the if condition "less than zero"
##########
gobblin-completeness/src/main/java/org/apache/gobblin/completeness/verifier/KafkaAuditCountVerifier.java:
##########
@@ -217,7 +217,7 @@ private static void validateTierCounts(String datasetName,
long beginInMillis, l
throw new IOException(String.format("Reference tier %s audit count
cannot be retrieved for dataset %s between %s and %s", refTier, datasetName,
beginInMillis, endInMillis));
}
long refCount = countsByTier.get(refTier);
- if(refCount <= 0) {
+ if (refCount < 0) {
Review Comment:
I think we should add a log.warn when refCount == 0. wdyt?
##########
gobblin-completeness/src/test/java/org/apache/gobblin/completeness/verifier/KafkaAuditCountVerifierTest.java:
##########
@@ -113,6 +113,20 @@ public void testTotalCountCompleteness() throws
IOException {
));
Assert.assertFalse(verifier.calculateCompleteness(topic, 0L, 0L)
.get(KafkaAuditCountVerifier.CompletenessType.TotalCountCompleteness));
+
+ // Check validation tiers for exceptions
+ client.setTierCounts(ImmutableMap.of(
+ SOURCE_TIER, 990L,
+ REFERENCE_TIERS, 0L,
+ TOTAL_COUNT_REF_TIER_0, 0L,
+ TOTAL_COUNT_REF_TIER_1, 0L
+ ));
+ try {
+ verifier.calculateCompleteness(topic, 0L, 0L)
+
.get(KafkaAuditCountVerifier.CompletenessType.TotalCountCompleteness);
Review Comment:
Your test case only covers TotalCountCompleteness. Add a new test for
ClassicCompleteness
--
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]
