zhengchenyu commented on PR #1413: URL: https://github.com/apache/incubator-uniffle/pull/1413#issuecomment-1888521299
> Emm... I take a look about current impl, I don't think we should take the tez prefix into the config key, you can refer the spark's RssClientConf . > > In spark's partial configOption, we just extract the spark's conf like spark.rss.client.type into RssConf, that means we could share some general configOption defined in the RssClientConf. So you just only create the TezClientConfig.java, which could define some tez specified configOptions, shareable configs could use defined in RssClientConf. cc @zhengchenyu @zuston I agree with you! If so, RssClientConf should be extended from RssClientConf (But for now, RssClientConf is extended form RssBaseConf ), and does not take the tez prefix into the config key. I think the tez prefix should be considered in the TezConfiguration. @lifeSo what about you? -- 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]
