-
Notifications
You must be signed in to change notification settings - Fork 930
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
Nacos support subscribe to all with '*' #2374
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2374 +/- ##
==========================================
- Coverage 44.22% 44.11% -0.12%
==========================================
Files 305 304 -1
Lines 18628 18594 -34
==========================================
- Hits 8239 8202 -37
- Misses 9526 9531 +5
+ Partials 863 861 -2
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
logger.Warnf("event listener game over.") | ||
return perrors.New("nacosRegistry is not available.") | ||
} | ||
services, err := nr.getAllSubscribeServiceNames() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a scheduled pulling task here? What if the service name list in Nacos server changes after we subscribed to '', for example, if a new service name is registered, will we still get notified by subscribing to ''?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be added later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what scenarios should subscribe to all services
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
logger.Warnf("event listener game over.") | ||
return perrors.New("nacosRegistry is not available.") | ||
} | ||
services, err := nr.getAllSubscribeServiceNames() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be added later.
No description provided.