lifeSo commented on code in PR #1413:
URL:
https://github.com/apache/incubator-uniffle/pull/1413#discussion_r1449160585
##########
client-tez/src/test/java/org/apache/tez/dag/app/TezRemoteShuffleManagerTest.java:
##########
@@ -224,7 +227,12 @@ public void testTezRemoteShuffleManagerSecure() {
secretManager.addTokenForJob(tokenIdentifier, sessionToken);
TezRemoteShuffleManager tezRemoteShuffleManager =
new TezRemoteShuffleManager(
- appId.toString(), sessionToken, conf, appId.toString(), client,
null);
+ appId.toString(),
+ sessionToken,
+ new TezClientConf(conf),
Review Comment:
I think set TEZ_RSS_TEST_MODE_ENABLE should be obvious, and not like hidden
in TezClientConf().
The normal case should read config from client side by Configuration, and
convert to TezClientConfig, so there need TezClientConfig(new Configuration()).
And no need new TezClientConfig(), because if so, there will be default
config and meanless.
So, I changed the test case to be:TezClientConf(new Configuration()), new
Configuration() just like conf from client side.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
