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

Master sync java enhancement #617

Merged

Conversation

KomachiSion
Copy link
Contributor

  • Set success as True when ack push success.
  • Same as java client, default close async update service by query.

@KomachiSion KomachiSion requested a review from binbin0325 May 25, 2023 02:48
@codecov-commenter
Copy link

Codecov Report

Patch coverage has no change and project coverage change: -0.57 ⚠️

Comparison is base (a23dd57) 30.66% compared to head (c6e087d) 30.09%.

❗ Current head c6e087d differs from pull request most recent head 848401a. Consider uploading reports for the commit 848401a to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #617      +/-   ##
==========================================
- Coverage   30.66%   30.09%   -0.57%     
==========================================
  Files          40       40              
  Lines        3059     3060       +1     
==========================================
- Hits          938      921      -17     
- Misses       2055     2072      +17     
- Partials       66       67       +1     
Impacted Files Coverage Δ
clients/naming_client/naming_client.go 34.67% <0.00%> (-0.69%) ⬇️
common/remote/rpc/server_request_handler.go 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -77,7 +77,9 @@ func NewNamingClient(nc nacos_client.INacosClient) (*NamingClient, error) {

naming.serviceProxy, err = NewNamingProxyDelegate(ctx, clientConfig, serverConfig, httpAgent, naming.serviceInfoHolder)

go NewServiceInfoUpdater(ctx, naming.serviceInfoHolder, clientConfig.UpdateThreadNum, naming.serviceProxy).asyncUpdateService()
if clientConfig.AsyncUpdateService {
Copy link
Member

Choose a reason for hiding this comment

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

这里是准备 默认关闭客户端更新服务实例的逻辑对吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Java2.1版本加了开关,默认是打开, 2.2开关默认是关闭的。

后续应该这个逻辑会保留,通过开关打开。

@binbin0325 binbin0325 merged commit 16975cb into nacos-group:master May 27, 2023
@KomachiSion KomachiSion deleted the Master-sync-java-enhancement branch May 29, 2023 01:24
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.

3 participants