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 2022/10/28 12:30:39 UTC
[GitHub] [ozone] smitajoshi12 opened a new pull request, #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
smitajoshi12 opened a new pull request, #3907:
URL: https://github.com/apache/ozone/pull/3907
What changes were proposed in this pull request?
When there is only one volume present in the cluster, the buckets dropdown does not list is getting displayed.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5112
## How was this patch tested?
Manually
--
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] dombizita commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
dombizita commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1013991054
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,14 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (((selectedVolumes && selectedVolumes.length != null) && (selectedVolumes.length === 2 && selectedVolumes[0].value === '*')) || (selectedVolumes.length === 1)){
Review Comment:
Is the `selectedVolumes.length != null` necessary? Later we are checking if the size is either 1 or 2.
```suggestion
if ((selectedVolumes && (selectedVolumes.length === 2 && selectedVolumes[0].value === '*')) || (selectedVolumes.length === 1)){
```
--
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] smitajoshi12 commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smitajoshi12 commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1013579527
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,15 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (selectedVolumes && selectedVolumes.length !== null && (selectedVolumes.length === 2 && selectedVolumes[0].value == '*') || (selectedVolumes.length === 1)){
+ let selectedVolume;
+ if (selectedVolumes.length === 1)
+ {
+ selectedVolume = selectedVolumes[0].value;
+ }
+ else {
+ selectedVolume = selectedVolumes[1].value;
+ }
Review Comment:
Indentation Issue Resolved.
--
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] smitajoshi12 commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smitajoshi12 commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1016207123
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,14 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (((selectedVolumes && selectedVolumes.length != null) && (selectedVolumes.length === 2 && selectedVolumes[0].value === '*')) || (selectedVolumes.length === 1)){
Review Comment:
Done Changes Suggested by Zita
--
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] smengcl merged pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smengcl merged PR #3907:
URL: https://github.com/apache/ozone/pull/3907
--
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] smitajoshi12 commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smitajoshi12 commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1014003960
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,14 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (((selectedVolumes && selectedVolumes.length != null) && (selectedVolumes.length === 2 && selectedVolumes[0].value === '*')) || (selectedVolumes.length === 1)){
Review Comment:
Yes Sometimes if length and selectedVolumes[0].value === '*' are both mandatory condition.
--
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] smengcl commented on pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smengcl commented on PR #3907:
URL: https://github.com/apache/ozone/pull/3907#issuecomment-1307732269
Thanks @smitajoshi12 for the patch. Thanks @dombizita for the review.
--
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] DaveTeng0 commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1010061619
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,15 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (selectedVolumes && selectedVolumes.length !== null && (selectedVolumes.length === 2 && selectedVolumes[0].value == '*') || (selectedVolumes.length === 1)){
Review Comment:
just curious, what does the * mean? and why it's the first element in selectedVolumes?
--
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] dombizita commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
dombizita commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1013273006
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,15 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (selectedVolumes && selectedVolumes.length !== null && (selectedVolumes.length === 2 && selectedVolumes[0].value == '*') || (selectedVolumes.length === 1)){
+ let selectedVolume;
+ if (selectedVolumes.length === 1)
+ {
+ selectedVolume = selectedVolumes[0].value;
+ }
+ else {
+ selectedVolume = selectedVolumes[1].value;
+ }
Review Comment:
Could you fix indentation in this section?
--
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] smitajoshi12 commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smitajoshi12 commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1010097345
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,15 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (selectedVolumes && selectedVolumes.length !== null && (selectedVolumes.length === 2 && selectedVolumes[0].value == '*') || (selectedVolumes.length === 1)){
Review Comment:
const allVolumesOption: IOption = {
label: 'All Volumes',
value: '*'
};
When we are selecting only one Volume then All volumes is getting selected by Default it is natural behaviour of dropdown so we are checking condition for One volume and multiple volume.
--
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] dombizita commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
dombizita commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1014024491
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,14 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (((selectedVolumes && selectedVolumes.length != null) && (selectedVolumes.length === 2 && selectedVolumes[0].value === '*')) || (selectedVolumes.length === 1)){
Review Comment:
I think the brackets wasn't at good place previously. So there is a `AND` after the `selectedVolumes`(like it was originally in the code) and after that inside the brackets there is an `OR`, where we need the length to be either 1 or 2. If we are checking that it either needs to be 1 or 2, why do we need the null check before that? Please check my suggestion in this comment.
```suggestion
if (selectedVolumes && ((selectedVolumes.length === 2 && selectedVolumes[0].value === '*') || (selectedVolumes.length === 1))){
```
--
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] DaveTeng0 commented on a diff in pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3907:
URL: https://github.com/apache/ozone/pull/3907#discussion_r1010811616
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/insights.tsx:
##########
@@ -89,8 +89,15 @@ export class Insights extends React.Component<Record<string, object>, IInsightsS
// selected buckets value should be reset to all buckets
let selectedBuckets = [allBucketsOption];
// Update bucket options only if one volume is selected
- if (selectedVolumes && selectedVolumes.length === 1) {
- const selectedVolume = selectedVolumes[0].value;
+ if (selectedVolumes && selectedVolumes.length !== null && (selectedVolumes.length === 2 && selectedVolumes[0].value == '*') || (selectedVolumes.length === 1)){
Review Comment:
i see!! 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] kerneltime commented on pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3907:
URL: https://github.com/apache/ozone/pull/3907#issuecomment-1297326435
@GeorgeJahad @SaketaChalamchala
--
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] smitajoshi12 commented on pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
smitajoshi12 commented on PR #3907:
URL: https://github.com/apache/ozone/pull/3907#issuecomment-1303164710
> Also I just checked and the `build-branch / build (8)` in the CI failed with this message: `Error: Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.12.0:npx (Build frontend) on project ozone-recon: Failed to run task: 'npx pnpm@5.18.9 run build' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]` Can you take a look at it? @smitajoshi12
@dombizita Hi Zita Now I can see build is success.
--
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] dombizita commented on pull request #3907: HDDS-5112. Recon Insights page does not list buckets when only one volume is present.
Posted by GitBox <gi...@apache.org>.
dombizita commented on PR #3907:
URL: https://github.com/apache/ozone/pull/3907#issuecomment-1302549175
Also I just checked and the `build-branch / build (8)` in the CI failed with this message:
```Error: Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.12.0:npx (Build frontend) on project ozone-recon: Failed to run task: 'npx pnpm@5.18.9 run build' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]```
Can you take a look at it? @smitajoshi12
--
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