You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "juncevich (via GitHub)" <gi...@apache.org> on 2024/08/30 13:42:21 UTC
[PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
juncevich opened a new pull request, #7138:
URL: https://github.com/apache/ozone/pull/7138
## What changes were proposed in this pull request?
Provide a one-liner summary of the changes in the PR **Title** field above.
It should be in the form of `HDDS-1234. Short summary of the change`.
Please describe your PR in detail:
Command `ozone admin pipeline close --all` close 0 pipelines. Should close all.
## What is the link to the Apache JIRA
[HDDS-11394](https://issues.apache.org/jira/browse/HDDS-11394)
## How was this patch tested?
Tested by unit tests
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #7138:
URL: https://github.com/apache/ozone/pull/7138#issuecomment-2325497090
@Montura Could you help take a look?
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "myskov (via GitHub)" <gi...@apache.org>.
myskov commented on PR #7138:
URL: https://github.com/apache/ozone/pull/7138#issuecomment-2362922170
@juncevich thanks for the patch. LGTM
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #7138:
URL: https://github.com/apache/ozone/pull/7138#issuecomment-2344756399
Thank you @juncevich for this change.
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "Montura (via GitHub)" <gi...@apache.org>.
Montura commented on code in PR #7138:
URL: https://github.com/apache/ozone/pull/7138#discussion_r1742386929
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ClosePipelineSubcommand.java:
##########
@@ -59,12 +59,22 @@ public void execute(ScmClient scmClient) throws IOException {
List<Pipeline> pipelineList = new ArrayList<>();
Predicate<? super Pipeline> predicate = replicationFilter.orElse(null);
- for (Pipeline pipeline : scmClient.listPipelines()) {
- boolean filterPassed = (predicate != null) && predicate.test(pipeline);
Review Comment:
I suggest just fix a condition without adding `if/else` block and more cycles:
```
boolean filterPassed = (predicate == null) || predicate.test(pipeline);
```
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "Montura (via GitHub)" <gi...@apache.org>.
Montura commented on code in PR #7138:
URL: https://github.com/apache/ozone/pull/7138#discussion_r1742386929
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ClosePipelineSubcommand.java:
##########
@@ -59,12 +59,22 @@ public void execute(ScmClient scmClient) throws IOException {
List<Pipeline> pipelineList = new ArrayList<>();
Predicate<? super Pipeline> predicate = replicationFilter.orElse(null);
- for (Pipeline pipeline : scmClient.listPipelines()) {
- boolean filterPassed = (predicate != null) && predicate.test(pipeline);
Review Comment:
I suggest just fix a condition without adding `if/else` block and more cycles:
```
boolean filterPassed = (predicate == null) || predicate.test(pipeline);
```
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "myskov (via GitHub)" <gi...@apache.org>.
myskov merged PR #7138:
URL: https://github.com/apache/ozone/pull/7138
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "Montura (via GitHub)" <gi...@apache.org>.
Montura commented on code in PR #7138:
URL: https://github.com/apache/ozone/pull/7138#discussion_r1742384383
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ClosePipelineSubcommand.java:
##########
@@ -59,12 +59,22 @@ public void execute(ScmClient scmClient) throws IOException {
List<Pipeline> pipelineList = new ArrayList<>();
Predicate<? super Pipeline> predicate = replicationFilter.orElse(null);
- for (Pipeline pipeline : scmClient.listPipelines()) {
- boolean filterPassed = (predicate != null) && predicate.test(pipeline);
- if (pipeline.getPipelineState() != Pipeline.PipelineState.CLOSED && filterPassed) {
- pipelineList.add(pipeline);
+ List<Pipeline> pipelines = scmClient.listPipelines();
Review Comment:
I suggest to fix a condition without adding `if/else` block:
```
boolean filterPassed = (predicate == null) || predicate.test(pipeline);
```
I checked, tests are working
--
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
Re: [PR] HDDS-11394. ozone admin pipeline close --all doesn't work without parameters [ozone]
Posted by "Montura (via GitHub)" <gi...@apache.org>.
Montura commented on code in PR #7138:
URL: https://github.com/apache/ozone/pull/7138#discussion_r1742384383
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/pipeline/ClosePipelineSubcommand.java:
##########
@@ -59,12 +59,22 @@ public void execute(ScmClient scmClient) throws IOException {
List<Pipeline> pipelineList = new ArrayList<>();
Predicate<? super Pipeline> predicate = replicationFilter.orElse(null);
- for (Pipeline pipeline : scmClient.listPipelines()) {
- boolean filterPassed = (predicate != null) && predicate.test(pipeline);
- if (pipeline.getPipelineState() != Pipeline.PipelineState.CLOSED && filterPassed) {
- pipelineList.add(pipeline);
+ List<Pipeline> pipelines = scmClient.listPipelines();
Review Comment:
I suggest to fix a condition without adding `if/else` block:
```
boolean filterPassed = (predicate == null) || predicate.test(pipeline);
```
I checked, tests are working
--
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