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

Return all subscriber when ip or port is null. #13008

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

zhouchunhai
Copy link
Contributor

Add a new param to support return all subscriber when ip or port is null.

Copy link

github-actions bot commented Jan 2, 2025

Thanks for your this PR. 🙏
Please check again for your PR changes whether contains any usage/api/configuration change such as Add new API , Add new configuration, Change default value of configuration.
If so, please add or update documents(markdown type) in docs/next/ for repository nacos-group/nacos-group.github.io


感谢您提交的PR。 🙏
请再次查看您的PR内容,确认是否包含任何使用方式/API/配置参数的变更,如:新增API新增配置参数修改默认配置等操作。
如果是,请确保在提交之前,在仓库nacos-group/nacos-group.github.io中的docs/next/目录下添加或更新文档(markdown格式)。

@codecov-commenter
Copy link

codecov-commenter commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.31%. Comparing base (595bdc9) to head (efc5ee4).

Files with missing lines Patch % Lines
.../naming/controllers/v2/ClientInfoControllerV2.java 33.33% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #13008      +/-   ##
=============================================
+ Coverage      72.27%   72.31%   +0.03%     
- Complexity      9998    10004       +6     
=============================================
  Files           1309     1309              
  Lines          41991    41993       +2     
  Branches        4405     4407       +2     
=============================================
+ Hits           30350    30366      +16     
+ Misses          9516     9501      -15     
- Partials        2125     2126       +1     
Files with missing lines Coverage Δ
.../naming/controllers/v2/ClientInfoControllerV2.java 68.96% <33.33%> (+12.82%) ⬆️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 595bdc9...efc5ee4. Read the comment docs.

@wuyfee
Copy link

wuyfee commented Jan 2, 2025

$\color{green}{SUCCESS}$
DETAILS
✅ - docker: success
✅ - deploy (standalone & cluster & standalone_auth): success
✅ - e2e-java-test (standalone & cluster & standalone_auth): success
✅ - e2e-go-test (standalone & cluster): success
✅ - e2e-cpp-test (standalone & cluster): success
✅ - e2e-csharp-test (standalone & cluster): success
✅ - e2e-nodejs-test (standalone & cluster): success
✅ - e2e-python-test (standalone & cluster): success
✅ - clean (standalone & cluster & standalone_auth): success

@wuyfee
Copy link

wuyfee commented Jan 2, 2025

$\color{green}{SUCCESS}$
DETAILS
✅ - docker: success
✅ - deploy (standalone & cluster & standalone_auth): success
✅ - e2e-java-test (standalone & cluster & standalone_auth): success
✅ - e2e-go-test (standalone & cluster): success
✅ - e2e-cpp-test (standalone & cluster): success
✅ - e2e-csharp-test (standalone & cluster): success
✅ - e2e-nodejs-test (standalone & cluster): success
✅ - e2e-python-test (standalone & cluster): success
✅ - clean (standalone & cluster & standalone_auth): success

@KomachiSion
Copy link
Collaborator

Where is the new params?

Copy link
Collaborator

@KomachiSion KomachiSion left a comment

Choose a reason for hiding this comment

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

No new params to judge whether filter all subs when ip or port is null.

@zhouchunhai
Copy link
Contributor Author

I think there is no need to add a new param. As described in the API doc, when ip or port is null, it should return al subs.
image

@Dario-s-j
Copy link

Dario-s-j commented Jan 3, 2025 via email

@KomachiSion
Copy link
Collaborator

OK, please help add some unit tests.

@Dario-s-j
Copy link

Dario-s-j commented Jan 6, 2025 via email

@wuyfee
Copy link

wuyfee commented Jan 7, 2025

$\color{red}{FAILURE}$
DETAILS
❌ - docker: failure
❌ - deploy (standalone & cluster & standalone_auth): skipped
❌ - e2e-java-test (standalone & cluster & standalone_auth): skipped
❌ - e2e-go-test (standalone & cluster): skipped
❌ - e2e-cpp-test (standalone & cluster): skipped
❌ - e2e-csharp-test (standalone & cluster): skipped
❌ - e2e-nodejs-test (standalone & cluster): skipped
❌ - e2e-python-test (standalone & cluster): skipped
❌ - clean (standalone & cluster & standalone_auth): failure

@zhouchunhai zhouchunhai changed the title Add a new param to support return all subscriber when ip or port is null. Return all subscriber when ip or port is null. Jan 7, 2025
@wuyfee
Copy link

wuyfee commented Jan 7, 2025

$\color{red}{FAILURE}$
DETAILS
❌ - docker: failure
❌ - deploy (standalone & cluster & standalone_auth): skipped
❌ - e2e-java-test (standalone & cluster & standalone_auth): skipped
❌ - e2e-go-test (standalone & cluster): skipped
❌ - e2e-cpp-test (standalone & cluster): skipped
❌ - e2e-csharp-test (standalone & cluster): skipped
❌ - e2e-nodejs-test (standalone & cluster): skipped
❌ - e2e-python-test (standalone & cluster): skipped
❌ - clean (standalone & cluster & standalone_auth): failure

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.

5 participants