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

Remove parameters from registryUrl to prevent config pollution #7189

Merged
merged 6 commits into from
Feb 22, 2021

Conversation

AlbumenJ
Copy link
Member

What is the purpose of the change

Fix #7188

Copy link
Member

@kylixs kylixs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's best to fix it based on 2.7.9-release for easy verification.

@chickenlj
Copy link
Contributor

We will release it in the next version.

@kylixs
Copy link
Member

kylixs commented Feb 18, 2021

We will release it in the next version.

This bug may affect users who set the group attribute of the registry. If there is no problem with the modified code, can it be merged into 2.7.9?

@chickenlj
Copy link
Contributor

I think there's no rush with this bugfix. I believe this is a regression of 2.7.8 or a few versions before. Considering that registry group feature is mostly used by some advanced users who most likely would stay in a stable version, there won't have many users facing this issue.

A new round of apache vote would take days to weeks, I think it's better to put it in the next release

@codecov-io
Copy link

codecov-io commented Feb 20, 2021

Codecov Report

Merging #7189 (e4a1524) into master (f7c6396) will decrease coverage by 0.02%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7189      +/-   ##
============================================
- Coverage     59.98%   59.96%   -0.03%     
+ Complexity      289      288       -1     
============================================
  Files          1004     1004              
  Lines         40016    40020       +4     
  Branches       5933     5937       +4     
============================================
- Hits          24005    23998       -7     
- Misses        13314    13318       +4     
- Partials       2697     2704       +7     
Impacted Files Coverage Δ Complexity Δ
...e/dubbo/rpc/cluster/directory/StaticDirectory.java 65.00% <0.00%> (ø) 0.00 <0.00> (ø)
...java/org/apache/dubbo/common/utils/ArrayUtils.java 18.18% <0.00%> (+1.51%) 0.00 <0.00> (ø)
...etadata/store/failover/StrategyMetadataReport.java 50.00% <0.00%> (ø) 0.00 <0.00> (ø)
...emoting/transport/grizzly/GrizzlyCodecAdapter.java 13.20% <0.00%> (+0.24%) 1.00 <0.00> (ø)
...apache/dubbo/rpc/protocol/redis/RedisProtocol.java 60.97% <0.00%> (ø) 7.00 <0.00> (ø)
...dubbo/rpc/cluster/directory/AbstractDirectory.java 70.27% <77.77%> (-1.61%) 0.00 <0.00> (ø)
...e/dubbo/registry/integration/DynamicDirectory.java 59.00% <100.00%> (ø) 0.00 <0.00> (ø)
...ache/dubbo/registry/multiple/MultipleRegistry.java 60.64% <100.00%> (+1.29%) 0.00 <0.00> (ø)
...bo/config/event/listener/LoggingEventListener.java 37.50% <0.00%> (-25.00%) 0.00% <0.00%> (ø%)
...mmon/threadpool/support/AbortPolicyWithReport.java 86.53% <0.00%> (-10.97%) 0.00% <0.00%> (ø%)
... and 27 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 6c08866...e4a1524. Read the comment docs.

@chickenlj chickenlj merged commit b0016ff into apache:master Feb 22, 2021
@AlbumenJ AlbumenJ mentioned this pull request Mar 1, 2021
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.

Configuration in Spring env Inherit problem
4 participants