dombizita commented on code in PR #6535:
URL: https://github.com/apache/ozone/pull/6535#discussion_r1608295035
##########
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) {
+ if (subpaths.length > limit || (subpaths.length > 0 && limit ===
MAX_DISPLAY_LIMIT)) {
subpaths = subpaths.slice(0, limit);
let topSize = 0;
- for (let i = 0; i < limit; ++i) {
+ for (let i = 0; limit === MAX_DISPLAY_LIMIT ? i < subpaths.length : i
< limit; ++i) {
Review Comment:
I think this could work wrong. What happens if the limit is set to 10, but
we only have 5 subpaths? In that case we would iterate until `i` is smaller
than 10? But we don't have that many elements in `subpaths`. Did I miss
something?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/diskUsage/diskUsage.tsx:
##########
@@ -265,8 +265,8 @@ export class DiskUsage extends
React.Component<Record<string, object>, IDUState>
updateDisplayLimit(e): void {
let res = -1;
- if (e.key === 'all') {
- res = Number.MAX_VALUE;
+ if (e.key === '30') {
+ res = Number.parseInt(e.key, 10);
} else {
res = Number.parseInt(e.key, 10);
}
Review Comment:
I don't understand this. With the changes in both cases (if `e.key` is
`'all'` and if it's `'30'`) we will set `res = Number.parseInt(e.key, 10);`
##########
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) {
+ if (subpaths.length > limit || (subpaths.length > 0 && limit ===
MAX_DISPLAY_LIMIT)) {
Review Comment:
@smitajoshi12 Could you explain why we need the other condition?
`(subpaths.length > 0 && limit === MAX_DISPLAY_LIMIT)` - we have any number
(not zero) of subpaths and the limit is set to 30, the max display limit. This
means that even even if we don't have more subpaths than the limit, we are
adding the `Other Objects`? Why is that needed?
--
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]
