abstractdog commented on code in PR #458:
URL: https://github.com/apache/tez/pull/458#discussion_r2832643997
##########
tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java:
##########
@@ -2429,7 +2430,10 @@ public static void main(String[] args) {
Objects.requireNonNull(appSubmitTimeStr,
ApplicationConstants.APP_SUBMIT_TIME_ENV + " is null");
- Configuration conf = new Configuration();
+ Configuration conf =
+
STANDALONE_ZOOKEEPER.name().equals(System.getenv(TezConstants.TEZ_FRAMEWORK_MODE))
+ ? new TezConfiguration()
+ : new Configuration();
Review Comment:
This should be kept somehow in FrameworkUtils, like
`FrameworkUtils.getInitialConfiguration()`
because we might want to hide the details of this "hack" from DAGAppMaster.
Why I call it a hack: it's not because the solution is bad, it's because of
the current design, which is:
1. we need a Configuration
2. for that we need to know the framework mode
3. the framework mode and classes needs a Configuration object already
`getFrameworkService(conf)`
so we cannot do anything better than to achieve original Configuration by
the value stored in environment...which is fine, but DAGAppMaster's already
overwhelmed logic should not contain such a thing
--
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]
