dombizita commented on code in PR #6535:
URL: https://github.com/apache/ozone/pull/6535#discussion_r1618640164
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/diskUsage/diskUsage.tsx:
##########
@@ -144,20 +145,19 @@ export class DiskUsage extends
React.Component<Record<string, object>, IDUState>
const dataSize = duResponse.size;
let subpaths: IDUSubpath[] = duResponse.subPaths;
- subpaths.sort((a, b) => (a.size < b.size) ? 1 : -1);
-
- // Only show top n blocks with the most DU,
- // other blocks are merged as a single block
- if (subpaths.length > limit) {
+ // This logic is for other objects for Max Limit 30 we are calculating
+ // Other Object Size = Total Size - Sum of all subpaths Size
Review Comment:
I don't understand this comment. I'd go with something like this:
```suggestion
// We need to calculate the size of "Other objects" in two cases:
// 1) If we have more subpaths listed, than the limit.
// 2) If the limit is set to the maximum limit (30) and we have any
number of subpaths. In this case we won't
// necessarily have "Other objects", but later we check if the other
objects's size is more than zero (we will have
// other objects if there are more than 30 subpaths, but we can't
check on that, as the response will always have
// 30 subpaths, but from the total size and the subpaths size we can
calculate it).
```
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/diskUsage/diskUsage.tsx:
##########
@@ -144,20 +145,19 @@ export class DiskUsage extends
React.Component<Record<string, object>, IDUState>
const dataSize = duResponse.size;
let subpaths: IDUSubpath[] = duResponse.subPaths;
- subpaths.sort((a, b) => (a.size < b.size) ? 1 : -1);
-
- // Only show top n blocks with the most DU,
- // other blocks are merged as a single block
- if (subpaths.length > limit) {
+ // This logic is for other objects for Max Limit 30 we are calculating
+ // Other Object Size = Total Size - Sum of all subpaths Size
+ if (subpaths.length >= limit || (subpaths.length > 0 && limit ===
MAX_DISPLAY_LIMIT)) {
Review Comment:
Why do we need the `>=` in the `subpaths.length >= limit` condition? If we
have 10 subpaths and 10 is the limit, we won't need "Other objects", right?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/diskUsage/diskUsage.tsx:
##########
@@ -265,12 +265,9 @@ export class DiskUsage extends
React.Component<Record<string, object>, IDUState>
updateDisplayLimit(e): void {
let res = -1;
- if (e.key === 'all') {
- res = Number.MAX_VALUE;
- } else {
+ if (e.key) {
res = Number.parseInt(e.key, 10);
}
-
Review Comment:
Why do we need all of this? Can't we just set the `res` like this?
`res = Number.parseInt(e.key, 10);`
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/diskUsage/diskUsage.tsx:
##########
@@ -549,7 +546,7 @@ export class DiskUsage extends
React.Component<Record<string, object>, IDUState>
</div>
<div className='dropdown-button'>
<Dropdown overlay={menu} placement='bottomCenter'>
- <Button>Display Limit: {(displayLimit ===
Number.MAX_VALUE) ? 'All' : displayLimit}</Button>
+ <Button>Display Limit: {(displayLimit ===
Number.MAX_VALUE) ? MAX_DISPLAY_LIMIT : displayLimit}</Button>
Review Comment:
I think this has some from the previous code. Isn't this enough?
```suggestion
<Button>Display Limit: {displayLimit}</Button>
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
