You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ashishkumar50 (via GitHub)" <gi...@apache.org> on 2023/07/20 11:50:03 UTC
[GitHub] [ozone] ashishkumar50 opened a new pull request, #5094: HDDS-9049. Use OM ServiceId from address in DeleteVolumeHandler.
ashishkumar50 opened a new pull request, #5094:
URL: https://github.com/apache/ozone/pull/5094
## What changes were proposed in this pull request?
OM ServiceId can be fetched from address and can be used to form Ozone address for fs URI. This can avoid giving service ID explicitly in option while running recursive delete volume command. Already this has been done for delete bucket command handler.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9049
## How was this patch tested?
Update existing integration test and verify.
--
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] adoroszlai commented on a diff in pull request #5094: HDDS-9049. Use OM ServiceId from address in DeleteVolumeHandler.
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5094:
URL: https://github.com/apache/ozone/pull/5094#discussion_r1269905442
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java:
##########
@@ -92,16 +85,17 @@ protected void execute(OzoneClient client, OzoneAddress address)
String volumeName = address.getVolumeName();
try {
if (bRecursive) {
- Collection<String> serviceIds = getConf().getTrimmedStringCollection(
- OZONE_OM_SERVICE_IDS_KEY);
- if (Strings.isNullOrEmpty(omServiceId)) {
- if (serviceIds.size() > 1) {
- out().printf("OmServiceID not provided, provide using " +
- "-id <OM_SERVICE_ID>%n");
- return;
- } else if (serviceIds.size() == 1) {
+ if (!Strings.isNullOrEmpty(address.getOmHost())) {
+ omServiceId = address.getOmHost();
+ } else {
+ Collection<String> serviceIds = getConf().getTrimmedStringCollection(
+ OZONE_OM_SERVICE_IDS_KEY);
+ if (serviceIds.size() == 1) {
// Only one OM service ID configured, we can use that
+ // If more than 1, it will fail in createClient step itself
omServiceId = serviceIds.iterator().next();
+ } else {
+ omServiceId = getConf().get(OZONE_OM_ADDRESS_KEY);
}
}
Review Comment:
This is same logic as in `DeleteBucketHandler`. Can you please extract and reuse?
--
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] adoroszlai commented on pull request #5094: HDDS-9049. Use OM service ID from Ozone address for recursive delete
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5094:
URL: https://github.com/apache/ozone/pull/5094#issuecomment-1646525317
Thanks @ashishkumar50 for the patch, @sadanand48 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] ashishkumar50 commented on a diff in pull request #5094: HDDS-9049. Use OM ServiceId from address in DeleteVolumeHandler.
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5094:
URL: https://github.com/apache/ozone/pull/5094#discussion_r1270193775
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/volume/DeleteVolumeHandler.java:
##########
@@ -92,16 +85,17 @@ protected void execute(OzoneClient client, OzoneAddress address)
String volumeName = address.getVolumeName();
try {
if (bRecursive) {
- Collection<String> serviceIds = getConf().getTrimmedStringCollection(
- OZONE_OM_SERVICE_IDS_KEY);
- if (Strings.isNullOrEmpty(omServiceId)) {
- if (serviceIds.size() > 1) {
- out().printf("OmServiceID not provided, provide using " +
- "-id <OM_SERVICE_ID>%n");
- return;
- } else if (serviceIds.size() == 1) {
+ if (!Strings.isNullOrEmpty(address.getOmHost())) {
+ omServiceId = address.getOmHost();
+ } else {
+ Collection<String> serviceIds = getConf().getTrimmedStringCollection(
+ OZONE_OM_SERVICE_IDS_KEY);
+ if (serviceIds.size() == 1) {
// Only one OM service ID configured, we can use that
+ // If more than 1, it will fail in createClient step itself
omServiceId = serviceIds.iterator().next();
+ } else {
+ omServiceId = getConf().get(OZONE_OM_ADDRESS_KEY);
}
}
Review Comment:
Thanks, extracted in common place and reused.
--
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] adoroszlai merged pull request #5094: HDDS-9049. Use OM service ID from Ozone address for recursive delete
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5094:
URL: https://github.com/apache/ozone/pull/5094
--
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] ashishkumar50 commented on pull request #5094: HDDS-9049. Use OM ServiceId from address in DeleteVolumeHandler.
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #5094:
URL: https://github.com/apache/ozone/pull/5094#issuecomment-1643796629
@adoroszlai @sadanand48 , Please help to 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