You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jianghuazhu (via GitHub)" <gi...@apache.org> on 2024/03/06 10:33:46 UTC
[PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
jianghuazhu opened a new pull request, #6337:
URL: https://github.com/apache/ozone/pull/6337
## What changes were proposed in this pull request?
There are some redundant strings in the HddsDispatcher and the purpose of this pr is to simplify them.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10473
## How was this patch tested?
You need to guarantee that the unit tests pass.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on code in PR #6337:
URL: https://github.com/apache/ozone/pull/6337#discussion_r1514263583
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -812,43 +811,54 @@ private static DNAction getAuditAction(Type cmdType) {
}
}
+ private static final String AUDIT_PARAM_CONTAINER_ID = "containerID";
Review Comment:
By convention the private static final constant fields are declared at the start of the class. Please move these declarations to around line 94.
I think the audit related methods and all these constants could be moved to a separate class/method in a separate issue to make HddsDispatcher shorter. What do you think @adoroszlai ?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on PR #6337:
URL: https://github.com/apache/ozone/pull/6337#issuecomment-1982842494
LGTM +1
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #6337:
URL: https://github.com/apache/ozone/pull/6337#issuecomment-1980569456
@adoroszlai , can you help take a look at this PR?
ci: https://github.com/jianghuazhu/ozone/actions/runs/8169369917
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on code in PR #6337:
URL: https://github.com/apache/ozone/pull/6337#discussion_r1515588632
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -812,43 +811,54 @@ private static DNAction getAuditAction(Type cmdType) {
}
}
+ private static final String AUDIT_PARAM_CONTAINER_ID = "containerID";
Review Comment:
Thanks @Galsza for the comments and review.
I've updated it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6337:
URL: https://github.com/apache/ozone/pull/6337#issuecomment-1986855470
Thanks @jianghuazhu for the patch, @Galsza for the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "jianghuazhu (via GitHub)" <gi...@apache.org>.
jianghuazhu commented on PR #6337:
URL: https://github.com/apache/ozone/pull/6337#issuecomment-1984941973
@adoroszlai , can you help review this pr?
Thanks.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6337:
URL: https://github.com/apache/ozone/pull/6337
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6337:
URL: https://github.com/apache/ozone/pull/6337#issuecomment-1985090843
@jianghuazhu please merge `master` into your branch to get the fix for compile error
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-10473. Simplify some strings in HddsDispatcher [ozone]
Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on code in PR #6337:
URL: https://github.com/apache/ozone/pull/6337#discussion_r1514263583
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -812,43 +811,54 @@ private static DNAction getAuditAction(Type cmdType) {
}
}
+ private static final String AUDIT_PARAM_CONTAINER_ID = "containerID";
Review Comment:
By convention the private static final constant fields are declared at the start of the class. Please move these declarations to around line 94.
I think the audit related methods and all these constants could be moved to a separate class/method to make HddsDispatcher shorter. What do you think @adoroszlai ?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org