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

sentinel-apache-dubbo-adapter not compatible dubbo 2.7.5 #1296

Merged
merged 2 commits into from
Feb 21, 2020

Conversation

zhaoyuguang
Copy link
Collaborator

resolve #1231
resolve #1292

@zhaoyuguang zhaoyuguang requested a review from sczyh30 February 21, 2020 08:14
@sczyh30 sczyh30 added area/integrations Issues or PRs related to integrations with open-source components to-review To review labels Feb 21, 2020
@sczyh30
Copy link
Member

sczyh30 commented Feb 21, 2020

The CI indicates test failure. Could you please resolve it?

@codecov-io
Copy link

Codecov Report

Merging #1296 into master will decrease coverage by 0.08%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1296      +/-   ##
============================================
- Coverage      42.8%   42.71%   -0.09%     
+ Complexity     1606     1602       -4     
============================================
  Files           344      344              
  Lines         10182    10186       +4     
  Branches       1380     1380              
============================================
- Hits           4358     4351       -7     
- Misses         5281     5290       +9     
- Partials        543      545       +2
Impacted Files Coverage Δ Complexity Δ
...entinel/adapter/dubbo/BaseSentinelDubboFilter.java 77.77% <50%> (-4.84%) 5 <0> (ø)
.../sentinel/adapter/dubbo/DubboAppContextFilter.java 80% <0%> (-20%) 2% <0%> (-1%)
...m/alibaba/csp/sentinel/log/DateFileLogHandler.java 54.54% <0%> (-3.04%) 6% <0%> (-1%)
...p/sentinel/datasource/consul/ConsulDataSource.java 69.56% <0%> (-2.9%) 7% <0%> (ø)
...tinel/slots/block/flow/param/ParamFlowChecker.java 52.7% <0%> (-2.71%) 28% <0%> (-1%)

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 5f203aa...30da30b. Read the comment docs.

@zhaoyuguang
Copy link
Collaborator Author

The CI indicates test failure. Could you please resolve it?

OKay, Please review again

@sczyh30
Copy link
Member

sczyh30 commented Feb 21, 2020

Nice. Have you tested the backward compatibility for Dubbo 2.7.5-?

@zhaoyuguang
Copy link
Collaborator Author

Nice. Have you tested the backward compatibility for Dubbo 2.7.5-?

The version of 2.7.3 , I tested, compatibility and passed

Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

@sczyh30 sczyh30 merged commit 2bdeccf into alibaba:master Feb 21, 2020
@sczyh30
Copy link
Member

sczyh30 commented Feb 21, 2020

Thanks for contributing!

@sczyh30 sczyh30 removed the to-review To review label Feb 21, 2020
@sczyh30 sczyh30 added this to the 1.7.2 milestone Feb 21, 2020
@seasidesky
Copy link
Contributor

ListenableFilter was deperated in dubbo 2.7.6 and onMessage method is renamed back to onResponse according to 2.7.6-release-note and Filter refactor, keep all callback methods inside Filter.Listener #5731
I thank it's better to seperate 2.7.3、2.7.4/5、2.7.6 implementation if sentinel need to support those different dubbo version

hughpearse pushed a commit to hughpearse/Sentinel that referenced this pull request Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/integrations Issues or PRs related to integrations with open-source components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dubbo2.7.5版本不兼容 sentinel-apache-dubbo-adapter dubbo2.7.5 ERROR
4 participants