HDDS-5465. Delete redundant code when set、add and remove bucket acl #2439
Conversation
| @@ -122,8 +122,7 @@ OMClientResponse onSuccess(OMResponse.Builder omResponse, | |||
| @Override | |||
| OMClientResponse onFailure(OMResponse.Builder omResponse, | |||
There was a problem hiding this comment.
You can just remove this onFailure implementation in each sub class.
There was a problem hiding this comment.
Thanks for review,I will delete it in each class
There was a problem hiding this comment.
Hi, @ChenSammi I have fixed it, PTAL! Thanks!
dineshchitlangia
left a comment
dineshchitlangia
left a comment
There was a problem hiding this comment.
@wzhallright LGTM
|
|
||
| if (operationResult) { | ||
| LOG.debug("Add acl: {} to path: {} success!", getAcls(), getPath()); | ||
| if (LOG.isDebugEnabled()) { |
There was a problem hiding this 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
There was a problem hiding this 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?
There was a problem hiding this comment.
You can file a new jira to track HDDS-2266, but it's unnecessary change here.
There was a problem hiding this comment.
OK, I will rollback it.
* master: (48 commits) HDDS-5514. Skip check for UNHEALTHY containers for datanode finalize. (apache#2469) HDDS-5279. OFS mkdir -p does not work when Volume is not pre-created (apache#2412) HDDS-5328. Remove delete container command from admin CLI (apache#2456) HDDS-5382. Increase default container report interval to 60 mins (apache#2363) HDDS-5378 Add APIs to retrieve Namespace Summary from Recon (apache#2417) HDDS-5466. Refactor BlockOutputStream. (apache#2442) HDDS-5465. Delete redundant code when set、add and remove bucket acl (apache#2439) HDDS-5184. Use separate DB profile for Datanodes. (apache#2214) HDDS-5494. Reduce retry in Kubernetes test (apache#2461) HDDS-5414. Data buffers incorrectly filtered for Ozone Insight (apache#2387) HDDS-5450. Avoid refresh pipeline for S3 headObject (apache#2431) HDDS-5500. New k3s version breaks kubernetes test (apache#2464) HDDS-5489. Install OS-specific flekszible (apache#2462) Multi-raft style placement with permutations for offline data generator (apache#2434) HDDS-5484. Intermittent failure in TestReplicationManager#testMovePrerequisites (apache#2454) HDDS-5443 Create and then recreate a bucket with a randomized name (apache#2436) HDDS-5492. Disable failing kubernetes test (apache#2459) HDDS-4330. Bootstrap new OM node (apache#1494) HDDS-5418. Let Recon send reregisterCommand to Datanodes if DatanodeDetails changed (apache#2392) HDDS-5479. s3g bucket list failed when there is non-english key name. (apache#2450) ...
4 participants
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?