-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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
check child is json or not in zookeeper.it will be continue if not. #14166
Conversation
@CrazyHZM hello,In the last commit, I only make the logger for warnings and did not change any logic. However, it still did not pass the checks.That is strange.I checked the logs, and it seems to be a timeout issue with Zookeeper. Could the success of the checks be related to network factors? |
...-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperRegistry.java
Show resolved
Hide resolved
@AlbumenJ I catch the exception,but i can't pass the CI/CD.I am trying to fix it. |
@AlbumenJ Take a look,please. |
...-registry-zookeeper/src/main/java/org/apache/dubbo/registry/zookeeper/ZookeeperRegistry.java
Outdated
Show resolved
Hide resolved
Also, pls add some test cases |
throw new Exception(child + "is not json"); | ||
} | ||
} catch (Exception e) { | ||
logger.warn(PROTOCOL_ERROR_DESERIALIZE, "", "", e.getMessage()); |
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.
Enhance log content here
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.
should i output more information here?
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.
Yes, for some human friendly comment
@AlbumenJ I had finish it,could you take a look please? |
throw new Exception(child + "is not json"); | ||
} | ||
} catch (Exception e) { | ||
logger.warn(PROTOCOL_ERROR_DESERIALIZE, "", "", e.getMessage()); |
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.
Yes, for some human friendly comment
|
What is the purpose of the change
catch the exception when the url can't be deserialization.related issue
Brief changelog
add the json check when ANY_VALUE.equals(url.getServiceInterface()) at ZookeeperRegistry#doSubscribe.
Verifying this change
the child will work normal if the child is json.It can still work when dubbo-admin run.
Checklist