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

refactor listenDirEvent #1589

Merged
merged 2 commits into from
Nov 19, 2021
Merged

refactor listenDirEvent #1589

merged 2 commits into from
Nov 19, 2021

Conversation

Mulavar
Copy link
Member

@Mulavar Mulavar commented Nov 15, 2021

  1. remove CLEAR label;
  2. remove register/unregister logic for watch children;
  3. reorder the zk dynamic configuration code
  4. adapt the gost

What this PR does:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


1. remove CLEAR label;
2. remove register/unregister logic for watch children;
3. reorder the zk dynamic configuration code
4. adapt the gost
@Mulavar Mulavar changed the title refactor listenDirEvent [WIP]refactor listenDirEvent Nov 15, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #1589 (19c0bb9) into 3.0 (1fdd429) will increase coverage by 0.14%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              3.0    #1589      +/-   ##
==========================================
+ Coverage   41.15%   41.29%   +0.14%     
==========================================
  Files         250      250              
  Lines       14277    14262      -15     
==========================================
+ Hits         5875     5890      +15     
+ Misses       7719     7689      -30     
  Partials      683      683              
Impacted Files Coverage Δ
registry/zookeeper/registry.go 1.23% <0.00%> (-0.03%) ⬇️
remoting/zookeeper/listener.go 0.00% <0.00%> (ø)
registry/nacos/listener.go 82.50% <0.00%> (+2.50%) ⬆️
metadata/report/delegate/delegate_report.go 34.43% <0.00%> (+7.94%) ⬆️

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 1fdd429...19c0bb9. Read the comment docs.

@Mulavar Mulavar changed the title [WIP]refactor listenDirEvent refactor listenDirEvent Nov 17, 2021
@@ -150,7 +150,11 @@ func (r *zkRegistry) InitListeners() {

// CreatePath creates the path in the registry center of zookeeper
func (r *zkRegistry) CreatePath(path string) error {
return r.ZkClient().Create(path)
err := r.ZkClient().Create(path)
if err != nil && err != zk.ErrNodeExists {
Copy link
Member

Choose a reason for hiding this comment

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

IMO, using errors.Is() is better than !=.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think != is just ok

@@ -80,15 +81,20 @@ func newZookeeperDynamicConfiguration(url *common.URL) (*zookeeperDynamicConfigu
logger.Errorf("zookeeper client start error ,error message is %v", err)
return nil, err
}
err = c.client.Create(c.rootPath)
if err != nil && err != zk.ErrNodeExists {
Copy link
Member

Choose a reason for hiding this comment

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

errors.Is

@justxuewei justxuewei merged commit a812919 into apache:3.0 Nov 19, 2021
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.

6 participants