phet commented on code in PR #3894:
URL: https://github.com/apache/gobblin/pull/3894#discussion_r1522061958
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/work/PriorJobStateWUProcessingSpec.java:
##########
@@ -78,42 +78,13 @@ public boolean isToDoJobLevelTiming() {
try {
FileSystem fs = Help.loadFileSystemForce(this);
JobState jobState = Help.loadJobStateUncached(this, fs);
- List<Tag<?>> tagsFromCurrentJob = this.getTags();
- String metricsSuffix = this.getMetricsSuffix();
- List<Tag<?>> tags = this.calcMergedTags(tagsFromCurrentJob,
metricsSuffix, jobState);
- return new EventSubmitterContext(tags, JobMetrics.NAMESPACE);
+ return new EventSubmitterContext.Builder()
+ .addTags(this.getTags())
+ .withGaaSJobProps(jobState.getProperties())
Review Comment:
is the invocation order critical here?
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/launcher/ProcessWorkUnitsJobLauncher.java:
##########
@@ -85,7 +85,9 @@ public void submitJob(List<WorkUnit> workunits) {
URI nameNodeUri = new URI(PropertiesUtils.getRequiredProp(this.jobProps,
GOBBLIN_TEMPORAL_JOB_LAUNCHER_ARG_NAME_NODE_URI));
// NOTE: `Path` is challenging for temporal to ser/de, but nonetheless
do pre-construct as `Path`, to pre-validate this prop string's contents
Path workUnitsDir = new
Path(PropertiesUtils.getRequiredProp(this.jobProps,
GOBBLIN_TEMPORAL_JOB_LAUNCHER_ARG_WORK_UNITS_DIR));
- EventSubmitterContext eventSubmitterContext = new
EventSubmitterContext(this.eventSubmitter);
+ EventSubmitterContext eventSubmitterContext = new
EventSubmitterContext.Builder()
+ .withEventSubmitter(eventSubmitter)
+ .build();
Review Comment:
actually, I did just suggest that you add a `Builder` ctor, but thinking
more, why force the use of this builder, when the old `EventSubmitterContext`
continues to serve this base case just fine?
--
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]
