feat(config): add root config api builder #1491

Mulavar · 2021-09-27T12:19:56Z

What this PR does:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

config/dubbo_bootstrap.go

LaurenceLiZhixin · 2021-09-27T13:40:55Z

config/application_config.go

@@ -113,3 +113,49 @@ func WithMetadataType(metadataType string) ApplicationConfigOpt {
 		ac.MetadataType = metadataType
 	}
 }
+
+type ApplicationConfigBuilder struct {
+	*ApplicationConfig


config *ApplicationConfig

codecov-commenter · 2021-09-27T13:48:57Z

Codecov Report

Merging #1491 (c5dfcd2) into config-api (c164f38) will decrease coverage by 0.54%.
The diff coverage is 11.42%.

❗ Current head c5dfcd2 differs from pull request most recent head 59ee8ea. Consider uploading reports for the commit 59ee8ea to get more accurate results

@@              Coverage Diff               @@
##           config-api    #1491      +/-   ##
==============================================
- Coverage       42.71%   42.17%   -0.55%     
==============================================
  Files             261      261              
  Lines           14957    15113     +156     
==============================================
- Hits             6389     6374      -15     
- Misses           7819     7994     +175     
+ Partials          749      745       -4

Impacted Files	Coverage Δ
config/application_config.go	`10.76% <0.00%> (-7.66%)`	⬇️
config/config_loader.go	`22.22% <0.00%> (ø)`
config/metadata_report_config.go	`28.00% <0.00%> (ø)`
config/root_config.go	`6.14% <0.00%> (-4.47%)`	⬇️
config/service_config.go	`8.92% <ø> (ø)`
config/service_discovery_config.go	`0.00% <0.00%> (-50.00%)`	⬇️
config/registry_config.go	`20.34% <1.81%> (-11.66%)`	⬇️
config/protocol_config.go	`8.33% <5.26%> (-26.56%)`	⬇️
config/dubbo_bootstrap.go	`64.06% <73.33%> (+7.19%)`	⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c164f38...59ee8ea. Read the comment docs.

AlexStocks · 2021-09-28T04:45:45Z

config/application_config.go

+	application *ApplicationConfig
+}
+
+func (acb *ApplicationConfigBuilder) Organization(organization string) *ApplicationConfigBuilder {


赋值函数为啥不在前面加上 Set?

一般这种builder模式的运用，在链式赋值时都不会使用set。
我理解是因为这种模式下没有 get 的场景，所以单独的 set 并不会给到更多的语义信息，可忽略。

可参考 curator，okhttp3 等框架。

感觉写成了 java。。。。。。。。。。。go 不是用以下形式吗？

&ApplicationConfigBuilder{ key1:value1, key2:value2, }

AlexStocks · 2021-09-28T04:46:17Z

config/root_config.go

+	rootConfig *RootConfig
+}
+
+func (rb *RootConfigBuilder) Application(application *ApplicationConfig) *RootConfigBuilder {


加上 Set 前缀

zouyx · 2021-09-29T03:16:54Z

config/application_config.go

+	application *ApplicationConfig
+}
+
+func (acb *ApplicationConfigBuilder) Organization(organization string) *ApplicationConfigBuilder {


感觉写成了 java。。。。。。。。。。。go 不是用以下形式吗？

&ApplicationConfigBuilder{ key1:value1, key2:value2, }

zouyx · 2021-09-29T03:18:38Z

config/config_loader.go

@@ -188,19 +188,19 @@ func RPCService(service common.RPCService) {
 // So you don't need to worry about the race condition
 func GetMetricConfig() *MetricConfig {
 	// todo
-	//if GetBaseConfig().MetricConfig == nil {
+	//if GetBaseConfig().Metric == nil {


todo 要不干掉？

解释一下上面的问题，
&ApplicationConfigBuilder{
key1:value1,
key2:value2,
}
是一种初始化方式，这个builder是另一个问题，是前段时间讨论后针对目前with的config api一个替换做法，我个人理解builder不算是java专有的吧，这个模式go里面也有用，但是java比较普遍，我们也是讨论了觉得这种方式可读性比with更强所以借鉴这种写法。

zouyx · 2021-09-29T03:19:27Z

config/protocol_config.go

+	return &ProtocolConfigBuilder{protocolConfig: &ProtocolConfig{}}
+}
+
+type ProtocolConfigBuilder struct {


这里也是。。。感觉写成了 java

zouyx · 2021-09-29T03:19:41Z

config/registry_config.go

@@ -284,3 +272,98 @@ func WithRegistryParams(params map[string]string) RegistryConfigOpt {
 		return config
 	}
 }
+
+func NewRegistryConfigBuilder() *RegistryConfigBuilder {


这里也是 java

zouyx · 2021-09-29T03:19:57Z

config/root_config.go

+	return &RootConfigBuilder{rootConfig: &RootConfig{}}
+}
+
+type RootConfigBuilder struct {


这里也是 java

zouyx · 2021-09-29T03:20:08Z

config/service_discovery_config.go

 	return nil
 }
+
+func NewServiceDiscoveryConfigBuilder() *ServiceDiscoveryConfigBuilder {


这里也 java

XiaoWeiKIN · 2021-09-29T03:56:22Z

hi，boy。关于dubboBootstrap 和configLoader我有一点疑问，dubboBootstrap 顾名思义是作为程序的引导类，一般来说引导类只有一个，但是似乎configLoader也是做引导程序使用的，这两个是单独对程序进行加载和启动吗？另外假设我使用api配置和外部化配置混用应该如何使用配置？ @Mulavar

…er config api

Mulavar · 2021-10-01T14:10:25Z

hi，boy。关于dubboBootstrap 和configLoader我有一点疑问，dubboBootstrap 顾名思义是作为程序的引导类，一般来说引导类只有一个，但是似乎configLoader也是做引导程序使用的，这两个是单独对程序进行加载和启动吗？另外假设我使用api配置和外部化配置混用应该如何使用配置？ @Mulavar

目前暂不考虑混用的场景，bootstrap 这块和 loader 的关系确实不太明晰，我们正在讨论，进一步是不是要合并，还是将程序入口拆分得更加合理 @XiaoWeiKIN

LaurenceLiZhixin · 2021-10-02T01:58:36Z

I thinks this pr is able to merge, and we would fix TODOs soon.

Mulavar force-pushed the config-api branch from ac7f80b to fda5130 Compare September 27, 2021 12:51

feat(config): add root&application config api

2c9435d

Mulavar force-pushed the config-api branch from fda5130 to 2c9435d Compare September 27, 2021 12:59

feat(config): add protocol config api

48ce4c8

LaurenceLiZhixin reviewed Sep 27, 2021

View reviewed changes

config/dubbo_bootstrap.go Show resolved Hide resolved

LaurenceLiZhixin reviewed Sep 27, 2021

View reviewed changes

cjphaha approved these changes Sep 27, 2021

View reviewed changes

feat(config): remove structure embedding

b73687a

AlexStocks reviewed Sep 28, 2021

View reviewed changes

feat(config): add registry config api builder

44fcd27

Mulavar force-pushed the config-api branch from ca0924a to 44fcd27 Compare September 28, 2021 12:50

feat(config): add service discovery config api builder

c5dfcd2

zouyx reviewed Sep 29, 2021

View reviewed changes

dongjianhui03 added 2 commits October 1, 2021 15:00

feat(config): add prefix Set

a264187

feat(config): add consumer&provider&metric&metadata&remote_route&logg…

59ee8ea

…er config api

AlexStocks approved these changes Oct 1, 2021

View reviewed changes

LaurenceLiZhixin approved these changes Oct 2, 2021

View reviewed changes

LaurenceLiZhixin merged commit 9220fa4 into apache:config-api Oct 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config): add root config api builder #1491

feat(config): add root config api builder #1491

Mulavar commented Sep 27, 2021

LaurenceLiZhixin Sep 27, 2021

codecov-commenter commented Sep 27, 2021 •

edited

Loading

AlexStocks Sep 28, 2021

Mulavar Sep 28, 2021

Mulavar Sep 28, 2021

zouyx Sep 29, 2021

AlexStocks Sep 28, 2021

zouyx Sep 29, 2021

zouyx Sep 29, 2021

Mulavar Sep 29, 2021

zouyx Sep 29, 2021

zouyx Sep 29, 2021

zouyx Sep 29, 2021

zouyx Sep 29, 2021

XiaoWeiKIN commented Sep 29, 2021

Mulavar commented Oct 1, 2021

LaurenceLiZhixin commented Oct 2, 2021

feat(config): add root config api builder #1491

feat(config): add root config api builder #1491

Conversation

Mulavar commented Sep 27, 2021

Choose a reason for hiding this comment

codecov-commenter commented Sep 27, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiaoWeiKIN commented Sep 29, 2021

Mulavar commented Oct 1, 2021

LaurenceLiZhixin commented Oct 2, 2021

codecov-commenter commented Sep 27, 2021 •

edited

Loading