-
Notifications
You must be signed in to change notification settings - Fork 26.6k
fix: For idl stub service, use idl service name for discovery (#15365) #15449
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #15449 +/- ##
============================================
+ Coverage 60.74% 60.90% +0.15%
- Complexity 10934 11444 +510
============================================
Files 1887 1888 +1
Lines 86242 86332 +90
Branches 12927 12946 +19
============================================
+ Hits 52391 52582 +191
+ Misses 28377 28313 -64
+ Partials 5474 5437 -37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
could you submit a new pr at https://github.com/apache/dubbo-samples to add a new sample module to confirm this pr? |
|
i will test it. |
|
@heliang666s @oxsean PTAL |
|
version compatibility should also be considered.
|
Given issue #15365, this incompatibility should only occur in interface-level service discovery scenarios where package and java_package differ. And when users want to migrate to application-level service discovery, they will also encounter this compatibility issue. Based on the official documentation (official documentation), it clearly states that package should be used for service discovery, which is why I believe this should be treated as a bug fix. So maybe I should only change this in application-level service discovery, but leave the interface-level service discovery as it is to maintain compatibility with existing clients. Since I'm relatively new to Dubbo, what would you recommend as the most appropriate way to handle this compatibility issue while still fixing the underlying problem? |
|
In my opinion, It is best to provide users with an option, just like |
|
I looked at the source code, and if i make this change, it would require modifications in many places... |
|
I found that the serviceKey is not only used for service discovery, but also for routing and circuit breaking. Therefore, using the How about adding a boolean value to indicate whether to use the IDL package name as the serviceKey? This way, existing services do not need to be changed, and new services can use the IDL package name as the serviceKey. |
What is the purpose of the change?
fix: For idl stub service, use idl service name for discovery (#15365)
Checklist