You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "zuston (via GitHub)" <gi...@apache.org> on 2023/11/09 05:49:50 UTC

[I] [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig [incubator-uniffle]

zuston opened a new issue, #1302:
URL: https://github.com/apache/incubator-uniffle/issues/1302

   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
   
   
   ### Search before asking
   
   - [X] I have searched in the [issues](https://github.com/apache/incubator-uniffle/issues?q=is%3Aissue) and found no similar issues.
   
   
   ### What would you like to be improved?
   
   We will use the `ConfigOption` to unify all client and server config style. But currently, there are some original code using the ConfigEntry, which is necessary to replace by the `ConfigEntry`
   
   ### How should we improve?
   
   _No response_
   
   ### Are you willing to submit PR?
   
   - [ ] Yes I am willing to submit a PR!


-- 
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: issues-unsubscribe@uniffle.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [I] [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig [uniffle]

Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston closed issue #1302: [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig
URL: https://github.com/apache/uniffle/issues/1302


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [I] [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig [incubator-uniffle]

Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on issue #1302:
URL: https://github.com/apache/incubator-uniffle/issues/1302#issuecomment-1809363087

   @zuston i am new here, let me have a try


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [I] [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig [incubator-uniffle]

Posted by "zhengchenyu (via GitHub)" <gi...@apache.org>.
zhengchenyu commented on issue #1302:
URL: https://github.com/apache/incubator-uniffle/issues/1302#issuecomment-1886598961

   Do we need to finalize the design about #1302 #1303 #1304 ?
    
   RssSparkConfig/RssTezConfig/RssMRConfig or SparkConf/Configuration need to spark/tez/mr prefix? 
   And how about RssClientConfig? Do we need RssSparkConfig/RssTezConfig/RssMRConfig to be extend from RssClientConfig?
   
   @zuston @jerqi @lifeSo Can you give us some suggesstion?
   
   


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [I] [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig [incubator-uniffle]

Posted by "lifeSo (via GitHub)" <gi...@apache.org>.
lifeSo commented on issue #1302:
URL: https://github.com/apache/incubator-uniffle/issues/1302#issuecomment-1887735810

   > Do we need to finalize the design about #1302 #1303 #1304 ?
   > 
   > RssSparkConfig/RssTezConfig/RssMRConfig or SparkConf/Configuration need to spark/tez/mr prefix? And how about RssClientConfig? Do we need RssSparkConfig/RssTezConfig/RssMRConfig to be extend from RssClientConfig?
   > 
   > @zuston @jerqi @lifeSo Can you give us some suggesstion?
   
   I think at least TezClientConf need 'tez.' prefix, because there is some plase can not direct use TezClientConf.get(...).
   Instead, it use as TezClientConf.someconf.key() and TezClientConf.someconf.defaultValue() where use Configuration.


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org


Re: [I] [Improvement] Replace ConfigEntry with ConfigOption in RssSparkConfig [incubator-uniffle]

Posted by "zhengchenyu (via GitHub)" <gi...@apache.org>.
zhengchenyu commented on issue #1302:
URL: https://github.com/apache/incubator-uniffle/issues/1302#issuecomment-1888334926

   @lifeSo
   
   We just need to reach some consensus. Then develop MR/Tez/Spark code based on this consensus.
   
   This is my idea. Since TezConfiguration/SparkConf is start with "tez."/"spark.", then RssConfig is not start with "tez."/"spark.".
   Then if TezClientConf is extended from RssConfig, TezClientConf/SparkConf should not start with "tez."/"spark.".
   
   If so, TezClientConf should be extended from TezConfiguration. SparkClientConf should be extended from SparkConf.
   RssClientConfig should be extended from RssConfig.
   TezClientConf can be transient to RssClientConfig.
   
   


-- 
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: issues-unsubscribe@uniffle.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org