You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "CAICAIIs (via GitHub)" <gi...@apache.org> on 2026/05/16 03:55:30 UTC
[PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
CAICAIIs opened a new pull request, #3326:
URL: https://github.com/apache/dubbo-go/pull/3326
## What
Continue #3204 by removing the legacy `config.GetConsumerConfig()` timeout fallback from protocol invokers.
This PR updates:
- `protocol/dubbo`
- `protocol/jsonrpc`
Both now read request timeout in this order:
1. URL `timeout` param
2. URL `ConsumerConfigKey` attribute
3. `global.DefaultConsumerConfig().RequestTimeout`
## Why
After #3320, `global / InstanceOptions` should be the runtime source of truth, and `config.RootConfig` should only remain as a legacy mirror/input adapter.
These two protocol packages only used `config.GetConsumerConfig().RequestTimeout` as a fallback. Using `global.DefaultConsumerConfig()` keeps the same default value (`3s`) while removing the direct runtime dependency on `config`.
## Tests
- `go test ./protocol/dubbo ./protocol/jsonrpc -run 'TestNewDubboInvokerUses|TestJsonrpcProtocolRefer' -count=1`
- `go test ./protocol/dubbo ./protocol/jsonrpc -count=1`
- `go test ./...`
### Checklist
- [x] I confirm the target branch is `develop`
- [x] Code has passed local testing
- [x] I have added tests that prove my fix is effective or that my feature works
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4465463186
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "CAICAIIs (via GitHub)" <gi...@apache.org>.
CAICAIIs commented on code in PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#discussion_r3259129419
##########
protocol/dubbo/dubbo_invoker.go:
##########
@@ -57,10 +56,9 @@ type DubboInvoker struct {
// NewDubboInvoker constructor
func NewDubboInvoker(url *common.URL, client *remoting.ExchangeClient) *DubboInvoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := constant.DefaultConsumerRequestTimeout
Review Comment:
确实，已更改
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4469728528
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#discussion_r3254194122
##########
protocol/jsonrpc/jsonrpc_protocol.go:
##########
@@ -80,18 +79,17 @@ func (jp *JsonrpcProtocol) Export(invoker base.Invoker) base.Exporter {
// Refer a remote JSON PRC service from registry
func (jp *JsonrpcProtocol) Refer(url *common.URL) base.Invoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := global.DefaultConsumerConfig().RequestTimeout
if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
- if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok {
+ if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok && consumerConf.RequestTimeout != "" {
rt = consumerConf.RequestTimeout
Review Comment:
`global.DefaultConsumerConfig()` allocates a new `ConsumerConfig` (including `References` map) on every `Refer` call, but here it’s only used to read the default timeout string. Consider avoiding per-invoker allocations by deferring the call until needed (only when neither URL timeout nor ConsumerConfig attribute provides a value), or by caching just the default timeout string at package scope / exposing a lightweight `global.DefaultRequestTimeout` constant.
##########
protocol/dubbo/dubbo_invoker.go:
##########
@@ -57,10 +56,9 @@ type DubboInvoker struct {
// NewDubboInvoker constructor
func NewDubboInvoker(url *common.URL, client *remoting.ExchangeClient) *DubboInvoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := global.DefaultConsumerConfig().RequestTimeout
if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
- if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok {
+ if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok && consumerConf.RequestTimeout != "" {
rt = consumerConf.RequestTimeout
}
Review Comment:
`global.DefaultConsumerConfig()` creates a new config (and allocates a `References` map) each time `NewDubboInvoker` is called, but this code only needs the default timeout string. To reduce allocations when creating many invokers, consider computing the default timeout lazily (only if URL param and consumer attribute are absent) or caching the default timeout string in a package-level variable.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4473892320
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4469829118
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4473934496
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4473757340
上面雨哥的那两个评论难道不就是应该这样吗
rt先赋默认值
如果用户配置不为空则用用户的配置赋值给rt
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4473877575
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#discussion_r3256118385
##########
protocol/dubbo/dubbo_invoker.go:
##########
@@ -57,10 +56,9 @@ type DubboInvoker struct {
// NewDubboInvoker constructor
func NewDubboInvoker(url *common.URL, client *remoting.ExchangeClient) *DubboInvoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := constant.DefaultConsumerRequestTimeout
Review Comment:
用`global.DefaultConsumerConfig().RequestTimeout`难道不比现在直观吗😂
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "CAICAIIs (via GitHub)" <gi...@apache.org>.
CAICAIIs commented on code in PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#discussion_r3259129419
##########
protocol/dubbo/dubbo_invoker.go:
##########
@@ -57,10 +56,9 @@ type DubboInvoker struct {
// NewDubboInvoker constructor
func NewDubboInvoker(url *common.URL, client *remoting.ExchangeClient) *DubboInvoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := constant.DefaultConsumerRequestTimeout
Review Comment:
确实，已更改
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4465483563
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3326?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 52.32%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`bce69d3`](https://app.codecov.io/gh/apache/dubbo-go/commit/bce69d328acb9c60f626cd0583fcbc920e0cb573?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 787 commits behind head on develop.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3326 +/- ##
===========================================
+ Coverage 46.76% 52.32% +5.56%
===========================================
Files 295 492 +197
Lines 17172 37761 +20589
===========================================
+ Hits 8031 19760 +11729
- Misses 8287 16398 +8111
- Partials 854 1603 +749
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-go/pull/3326?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#discussion_r3254210334
##########
protocol/jsonrpc/jsonrpc_protocol.go:
##########
@@ -80,18 +79,16 @@ func (jp *JsonrpcProtocol) Export(invoker base.Invoker) base.Exporter {
// Refer a remote JSON PRC service from registry
func (jp *JsonrpcProtocol) Refer(url *common.URL) base.Invoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := ""
Review Comment:
[P0] 同样的问题这里又出现了一次：`global.DefaultConsumerConfig()` 只会给出硬编码默认值，不会带上用户通过配置文件/配置中心加载的 `consumer.request-timeout`。结果是 JSON-RPC refer 路径在 URL 未显式带 timeout 时会错误地退回 3s。建议沿用当前生效的 consumer 配置，并补一条覆盖全局 consumer timeout 的测试。
##########
protocol/dubbo/dubbo_invoker.go:
##########
@@ -57,10 +56,9 @@ type DubboInvoker struct {
// NewDubboInvoker constructor
func NewDubboInvoker(url *common.URL, client *remoting.ExchangeClient) *DubboInvoker {
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt := config.GetConsumerConfig().RequestTimeout
+ rt := ""
Review Comment:
[P0] 这里把 fallback 从 `config.GetConsumerConfig()` 改成了 `global.DefaultConsumerConfig()`，会直接绕过用户已经加载到 root config 里的 `consumer.request-timeout`。只要 URL 上没有 `ConsumerConfigKey` attribute，这里就固定回退到 3s，导致现有全局 consumer 超时配置失效。建议继续读取当前生效的 consumer 配置，只移除 legacy API 兼容层，不要把全局配置旁路掉；同时补一条“仅配置 root consumer timeout 且 URL 无 attribute” 的回归测试。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4470047207
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3326&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3326&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3326&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3326)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(config): remove legacy protocol timeout fallback [dubbo-go]
Posted by "CAICAIIs (via GitHub)" <gi...@apache.org>.
CAICAIIs commented on PR #3326:
URL: https://github.com/apache/dubbo-go/pull/3326#issuecomment-4473873882
> 上面雨哥的那两个评论难道不就是应该这样吗 rt先赋默认值 如果用户配置不为空则用用户的配置赋值给rt
之前是 protocol 层的 fallback 依赖 `GetParamDuration` 的隐式默认值确实不够优雅，现在我改成先使用显式默认超时时间，再由有效的 consumer 配置覆盖
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org