phet commented on code in PR #3892:
URL: https://github.com/apache/gobblin/pull/3892#discussion_r1520856867
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/proc/LaunchDagProc.java:
##########
@@ -45,9 +45,9 @@ public class LaunchDagProc extends
DagProc<Optional<Dag<JobExecutionPlan>>, Opti
public LaunchDagProc(LaunchDagTask launchDagTask) {
this.launchDagTask = launchDagTask;
this.orchestrationDelayCounter = new AtomicLong(0);
- ContextAwareGauge<Long> orchestrationDelayMetric =
this.metricContext.newContextAwareGauge
+ ContextAwareGauge<Long> orchestrationDelayMetric =
metricContext.newContextAwareGauge
(ServiceMetricNames.FLOW_ORCHESTRATION_DELAY,
orchestrationDelayCounter::get);
- this.metricContext.register(orchestrationDelayMetric);
+ metricContext.register(orchestrationDelayMetric);
Review Comment:
why change how it's referenced... isn't `metricContext` still an instance
member?
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java:
##########
@@ -120,7 +122,7 @@ public class GobblinServiceJobScheduler extends
JobScheduler implements SpecCata
protected final Optional<UserQuotaManager> quotaManager;
protected final Optional<FlowTriggerHandler> flowTriggerHandler;
@Getter
- protected final Map<String, Spec> scheduledFlowSpecs;
+ protected final Map<String, FlowSpec> scheduledFlowSpecs;
Review Comment:
aah, ok, I see - so essentially just following more widely the
specific-derived-class (`FlowSpec`) typing instituted therein
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/Orchestrator.java:
##########
@@ -111,14 +111,13 @@ public class Orchestrator implements SpecCatalogListener,
Instrumentable {
@Getter
private final SharedFlowMetricsSingleton sharedFlowMetricsSingleton;
- private final ClassAliasResolver<SpecCompiler> aliasResolver;
Review Comment:
good call to rescope as method level!
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/utils/FlowCompilationValidationHelper.java:
##########
@@ -59,6 +69,30 @@ public final class FlowCompilationValidationHelper {
private final FlowStatusGenerator flowStatusGenerator;
private final boolean isFlowConcurrencyEnabled;
+ @Inject
+ public FlowCompilationValidationHelper(Config config,
SharedFlowMetricsSingleton sharedFlowMetricsSingleton,
+ UserQuotaManager userQuotaManager, FlowStatusGenerator
flowStatusGenerator) {
+ try {
+ String specCompilerClassName =
ServiceConfigKeys.DEFAULT_GOBBLIN_SERVICE_FLOWCOMPILER_CLASS;
+ if
(config.hasPath(ServiceConfigKeys.GOBBLIN_SERVICE_FLOWCOMPILER_CLASS_KEY)) {
+ specCompilerClassName =
config.getString(ServiceConfigKeys.GOBBLIN_SERVICE_FLOWCOMPILER_CLASS_KEY);
Review Comment:
isn't there a method in `ConfigUtils` (or similar utility) that we could
leverage to provide such fallback to a default?
--
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]
