Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consumer params should not be overwritten, fix 9922. #10011

Closed
wants to merge 1 commit into from

Conversation

jefferson-chern
Copy link

@jefferson-chern jefferson-chern commented May 7, 2022

What is the purpose of the change

fixes #9922

Brief changelog

删除了消费者端从提供者端合并参数的操作。

Verifying this change

Checklist

  • Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
  • Add some description to dubbo-website project if you are requesting to add a feature.
  • GitHub Actions works fine on your own branch.
  • If this contribution is large, please follow the Software Donation Guide.

@jefferson-chern
Copy link
Author

我这两天翻阅了跟这行代码相关的issue #5971 #8325 #6426 #6926 和PR #9147 #6666 ,觉得 这张图表示的有些道理,消费者不应该感知提供者的这些配置,这些本来就是消费者自己提供的。所以删除这行代码比较合理,当然还会引起 #8325 之类的问题,但是这些问题是不是应该解决。

@chickenlj
Copy link
Contributor

当然还会引起 #8325 之类的问题,但是这些问题是不是应该解决。

这是两个相互矛盾的诉求,我们只能有优先级取舍。我认为我们应该遵循图中描述的,严格区分cluster层和isntance层的参数覆盖关系。

@chickenlj
Copy link
Contributor

非常感谢。但是当前 master 分之的话,我们更倾向于合并这个方案 https://github.com/apache/dubbo/pull/9933,因为它本身的兼容性问题更少。

但在 3.0 中我们会遵循你这里提到的思路,避免覆盖consumer端参数,请协助检查 3.0 相关逻辑。

@chickenlj chickenlj closed this May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants