You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/08/08 16:14:55 UTC
[GitHub] [accumulo] rohanshankar opened a new pull request, #2858: Added narrower exceptions to initializeMetrics method
rohanshankar opened a new pull request, #2858:
URL: https://github.com/apache/accumulo/pull/2858
Threw NPEs and Exceptions with descriptions to the initializeMetrics method in the MetrricsUtil.java class
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] rohanshankar commented on pull request #2858: Added narrower exceptions to initializeMetrics method
Posted by GitBox <gi...@apache.org>.
rohanshankar commented on PR #2858:
URL: https://github.com/apache/accumulo/pull/2858#issuecomment-1208362705
> Was this PR related to a specific task? If so, please mention the issue in the PR description somewhere. Based on Dave's above comments about the misunderstanding, I'm going to close this. If I understand this correctly, this task is going to be rolled into a slightly larger change that addresses this and several other minor issues all at once. So, there's probably nothing more to do here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on pull request #2858: Added narrower exceptions to initializeMetrics method
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2858:
URL: https://github.com/apache/accumulo/pull/2858#issuecomment-1208361689
Was this PR related to a specific task? If so, please mention the issue in the PR description somewhere.
Based on Dave's above comments about the misunderstanding, I'm going to close this. If I understand this correctly, this task is going to be rolled into a slightly larger change that addresses this and several other minor issues all at once. So, there's probably nothing more to do here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii closed pull request #2858: Added narrower exceptions to initializeMetrics method
Posted by GitBox <gi...@apache.org>.
ctubbsii closed pull request #2858: Added narrower exceptions to initializeMetrics method
URL: https://github.com/apache/accumulo/pull/2858
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a diff in pull request #2858: Added narrower exceptions to initializeMetrics method
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2858:
URL: https://github.com/apache/accumulo/pull/2858#discussion_r940427801
##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsUtil.java:
##########
@@ -76,6 +77,8 @@ private static void initializeMetrics(boolean enabled, boolean jvmMetricsEnabled
if (address != null) {
tags.add(Tag.of("Address", address.toString()));
+ } else {
+ throw new NullArgumentException();
Review Comment:
Why not IllegalArgumentException with a description - no need to add a new dependency for a standard exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] rohanshankar commented on pull request #2858: Added narrower exceptions to initializeMetrics method
Posted by GitBox <gi...@apache.org>.
rohanshankar commented on PR #2858:
URL: https://github.com/apache/accumulo/pull/2858#issuecomment-1208342049
> ptions being thrown in the method body
Oh ok, sorry about that. Just to clarify, I should replace the throws Exception in the method declaration with a more specific exception that encompasses the possible exceptions thrown by anything within the method body
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org