phet commented on code in PR #3896:
URL: https://github.com/apache/gobblin/pull/3896#discussion_r1550882395
##########
gobblin-service/src/main/java/org/apache/gobblin/service/monitoring/KafkaJobStatusMonitor.java:
##########
@@ -234,17 +248,14 @@ protected void
processMessage(DecodeableKafkaRecord<byte[],byte[]> message) {
}
/**
- * Persist job status to the underlying {@link StateStore}.
- * It fills missing fields in job status and also merge the fields with the
- * existing job status in the state store. Merging is required because we
- * do not want to lose the information sent by other GobblinTrackingEvents.
- * Returns an absent Optional if adding this state transitions the job
status of the job to final, otherwise returns
- * the updated job status wrapped inside an Optional.
- * It will also return an absent Optional if the job status was already
final before calling this method.
+ * It fills missing fields in job status and also merge the fields with the
existing job status in the state store.
+ * Merging is required because we do not want to lose the information sent
by other GobblinTrackingEvents.
+ * Returns a pair of current job status after update in this method and the
last previous state for this job wrapped
+ * inside an Optional.
* @throws IOException
*/
@VisibleForTesting
- static Optional<org.apache.gobblin.configuration.State>
addJobStatusToStateStore(org.apache.gobblin.configuration.State jobStatus,
+ static Pair<org.apache.gobblin.configuration.State,
Optional<org.apache.gobblin.configuration.State>>
updateJobStatus(org.apache.gobblin.configuration.State jobStatus,
Review Comment:
seems better encapsulation to calculate whether `isNewTransitionToFinal`
within and return `Pair<State, Boolean>`.
whenever previously `Optional.isPresent`, now `Pair<State, true>`
whenever previously `Optional.empty`, now `Pair<State, false>`
also, since not actually "updating" the state store, maybe name this
`recalcJobStatus` or `calcUpdatedJobStatus`?
--
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]
