You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/07/20 04:02:01 UTC
[GitHub] [ozone] wzhallright opened a new pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
wzhallright opened a new pull request #2439:
URL: https://github.com/apache/ozone/pull/2439
## What changes were proposed in this pull request?
I found that if the ACL operation of the bucket fails, onFailure has the same operation in OMBucketAddAclRequest、OMBucketRemoveAclRequest、OMBucketSetAclRequest.
So maybe we can move the onfailure operation to the parent class.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5465
## How was this patch tested?
--
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
[GitHub] [ozone] wzhallright commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
wzhallright commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r672973759
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -122,8 +122,7 @@ OMClientResponse onSuccess(OMResponse.Builder omResponse,
@Override
OMClientResponse onFailure(OMResponse.Builder omResponse,
Review comment:
Thanks for review,I will delete it in each 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: 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
[GitHub] [ozone] wzhallright commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
wzhallright commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r676250330
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -134,7 +127,9 @@ void onComplete(boolean operationResult, IOException exception,
exception, getOmRequest().getUserInfo()));
if (operationResult) {
- LOG.debug("Add acl: {} to path: {} success!", getAcls(), getPath());
+ if (LOG.isDebugEnabled()) {
Review comment:
Thanks for review! I saw it used in OMBucketSetAclRequest. It is introduced by HDDS-2266.
There are many class related to the patch, may be we can fix in separate JIRAs?
--
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
[GitHub] [ozone] ferhui commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
ferhui commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r676237618
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -134,7 +127,9 @@ void onComplete(boolean operationResult, IOException exception,
exception, getOmRequest().getUserInfo()));
if (operationResult) {
- LOG.debug("Add acl: {} to path: {} success!", getAcls(), getPath());
+ if (LOG.isDebugEnabled()) {
Review comment:
Is this for performance? Maybe there is no problem here.
It is recommended to use parameterized messages.
Please see http://www.slf4j.org/faq.html#logging_performance
--
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
[GitHub] [ozone] wzhallright commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
wzhallright commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r674582884
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -122,8 +122,7 @@ OMClientResponse onSuccess(OMResponse.Builder omResponse,
@Override
OMClientResponse onFailure(OMResponse.Builder omResponse,
Review comment:
Hi, @ChenSammi I have fixed it, PTAL! 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
[GitHub] [ozone] ChenSammi commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r672966958
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -122,8 +122,7 @@ OMClientResponse onSuccess(OMResponse.Builder omResponse,
@Override
OMClientResponse onFailure(OMResponse.Builder omResponse,
Review comment:
You can just remove this onFailure implementation in each sub 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: 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
[GitHub] [ozone] ferhui commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
ferhui commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r676268512
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -134,7 +127,9 @@ void onComplete(boolean operationResult, IOException exception,
exception, getOmRequest().getUserInfo()));
if (operationResult) {
- LOG.debug("Add acl: {} to path: {} success!", getAcls(), getPath());
+ if (LOG.isDebugEnabled()) {
Review comment:
You can file a new jira to track HDDS-2266, but it's unnecessary change 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: 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
[GitHub] [ozone] ChenSammi merged pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
ChenSammi merged pull request #2439:
URL: https://github.com/apache/ozone/pull/2439
--
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
[GitHub] [ozone] wzhallright commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
wzhallright commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r676272169
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -134,7 +127,9 @@ void onComplete(boolean operationResult, IOException exception,
exception, getOmRequest().getUserInfo()));
if (operationResult) {
- LOG.debug("Add acl: {} to path: {} success!", getAcls(), getPath());
+ if (LOG.isDebugEnabled()) {
Review comment:
OK, I will rollback 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
[GitHub] [ozone] wzhallright commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
wzhallright commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r672973759
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -122,8 +122,7 @@ OMClientResponse onSuccess(OMResponse.Builder omResponse,
@Override
OMClientResponse onFailure(OMResponse.Builder omResponse,
Review comment:
Thanks for review,I will delete it in each 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: 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
[GitHub] [ozone] wzhallright commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
wzhallright commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r676250330
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -134,7 +127,9 @@ void onComplete(boolean operationResult, IOException exception,
exception, getOmRequest().getUserInfo()));
if (operationResult) {
- LOG.debug("Add acl: {} to path: {} success!", getAcls(), getPath());
+ if (LOG.isDebugEnabled()) {
Review comment:
Thanks for review! I saw the usage in OMBucketSetAclRequest. It is introduced by HDDS-2266.
There are many class related to the patch, may be we can fix in separate JIRAs?
--
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
[GitHub] [ozone] ChenSammi commented on a change in pull request #2439: HDDS-5465. Delete redundant code when set、add and remove bucket acl
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2439:
URL: https://github.com/apache/ozone/pull/2439#discussion_r672966958
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAddAclRequest.java
##########
@@ -122,8 +122,7 @@ OMClientResponse onSuccess(OMResponse.Builder omResponse,
@Override
OMClientResponse onFailure(OMResponse.Builder omResponse,
Review comment:
You can just remove this onFailure implementation in each sub 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: 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