HDDS-2501. Sonar: Fix issues found in the ObjectEndpoint class. #198
Conversation
| LOG.error("Error in Initiate Multipart Upload Request for bucket: " + | ||
| bucket + ", key: " + key, ex); | ||
| LOG.error("Error in Initiate Multipart Upload Request for bucket: {}, " + | ||
| "key: ", bucket, key, ex); |
There was a problem hiding this comment.
Missing {} for parameter key
There was a problem hiding this comment.
Thanks for catching this, updating.
bharatviswa504
left a comment
bharatviswa504
left a comment
There was a problem hiding this comment.
One minor comment rest LGTM.
dineshchitlangia
left a comment
dineshchitlangia
left a comment
There was a problem hiding this comment.
+1 LGTM. Failures are unrelated. Thanks @swagle for the fix @bharatviswa504 for reviews.
| } else { | ||
| IOUtils.copy(body, ozoneOutputStream); | ||
| omMultipartCommitUploadPartInfo = ozoneOutputStream | ||
| .getCommitUploadPartInfo(); |
There was a problem hiding this comment.
It turns out this should be called only after ozoneOutputStream is closed (see #206).
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("range Header provided value is {}", rangeHeaderVal); | ||
| } |
There was a problem hiding this comment.
Wrapping every LOG.debug in an if increases method complexity and may trigger a new Sonar warning above a threshold (it does here). I think log statements are safe to call unconditionally (both for performance and for Sonar) as long as their arguments are not eagerly evaluated. Using placeholders and passing existing objects as arguments should be OK.
4 participants
What changes were proposed in this pull request?
Fix the sonarlint issues found in ObjectEndpoint class. The only ones not addressed: Logging and re-throwing Exception and TODOs. This is an Endpoint API class and hence should log errors unlike lower layers in the service.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-2501
How was this patch tested?
Only syntactic changes done, waiting on UT results.