Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functionality added for GCS delete multipart upload policy #18

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

neerajsinghal05
Copy link
Collaborator

Functionality added for GCS delete multipart upload policy

@@ -40,7 +45,8 @@ public class StorageClient {

private static Storage getStorageService() throws IOException {
return (null == storageService) ? StorageOptions.newBuilder().setProjectId(
PluginPropertyUtils.pluginProp(ConstantsUtil.PROJECT_ID)).build().getService() : storageService;
PluginPropertyUtils.pluginProp(ConstantsUtil.PROJECT_ID)).build().getService()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no change in code, revert them this should not be shown in PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

Copy link
Collaborator

@rahuldash171 rahuldash171 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address review comments

@@ -40,7 +45,8 @@ public class StorageClient {

private static Storage getStorageService() throws IOException {
return (null == storageService) ? StorageOptions.newBuilder().setProjectId(
PluginPropertyUtils.pluginProp(ConstantsUtil.PROJECT_ID)).build().getService() : storageService;
PluginPropertyUtils.pluginProp(ConstantsUtil.PROJECT_ID)).build().getService()
: storageService;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the writing format has been changed if there is no change ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

@@ -52,7 +58,7 @@ public static Page<Blob> listObjects(String bucketName) throws IOException {
}

public static Page<Blob> listObjectsWithPrefix(
String bucketName, String directoryPrefix) throws IOException {
String bucketName, String directoryPrefix) throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove space so that it can not be seen as a new line

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

return getStorageService().get(bucketName, blobName, Storage.BlobGetOption.fields(Storage.BlobField.values()));
throws StorageException, IOException {
return getStorageService().get(bucketName, blobName,
Storage.BlobGetOption.fields(Storage.BlobField.values()));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove new line , no change here too, work on spaces . format should not be changed for existing code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

public static Bucket getBucketMetadata(String bucketName) throws IOException {
return getStorageService().get(bucketName, Storage.BucketGetOption.fields(Storage.BucketField.values()));
return getStorageService().get(bucketName,
Storage.BucketGetOption.fields(Storage.BucketField.values()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here , dont change existing code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

@@ -87,10 +95,18 @@ public static Bucket createBucket(String bucketName) throws IOException {
}

public static Blob uploadObject(String bucketName, String objectName, String filePath)
throws IOException, URISyntaxException {
throws IOException, URISyntaxException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here , undo the changes

BlobInfo.newBuilder(BlobId.of(bucketName, objectName)).build(),
Files.readAllBytes(Paths.get(StorageClient.class.getResource("/" + filePath).toURI())));
BlobInfo.newBuilder(BlobId.of(bucketName, objectName)).build(),
Files.readAllBytes(Paths.get(StorageClient.class.getResource("/" + filePath).toURI())));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here , undo the changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change is not seen it is still seen as new line

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

ImmutableList.of(
new LifecycleRule(
LifecycleAction.newAbortIncompleteMPUploadAction(),
LifecycleCondition.newBuilder().setAge(age).build()))).build().update(); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the curly braces below and there should be a space between the final and this curly braces after you change .
LifecycleCondition.newBuilder().setAge(age).build()))).build().update();
}

}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve this .

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

ImmutableList.of(
new LifecycleRule(
LifecycleAction.newAbortIncompleteMPUploadAction(),
LifecycleCondition.newBuilder().setAge(age).build()))).build().update(); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve this .

BlobInfo.newBuilder(BlobId.of(bucketName, objectName)).build(),
Files.readAllBytes(Paths.get(StorageClient.class.getResource("/" + filePath).toURI())));
BlobInfo.newBuilder(BlobId.of(bucketName, objectName)).build(),
Files.readAllBytes(Paths.get(StorageClient.class.getResource("/" + filePath).toURI())));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change is not seen it is still seen as new line

@@ -40,7 +45,7 @@ public class StorageClient {

private static Storage getStorageService() throws IOException {
return (null == storageService) ? StorageOptions.newBuilder().setProjectId(
PluginPropertyUtils.pluginProp(ConstantsUtil.PROJECT_ID)).build().getService() : storageService;
PluginPropertyUtils.pluginProp(ConstantsUtil.PROJECT_ID)).build().getService() : storageService;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new line is seeen here too

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes has been done , as per the review comment

@neerajsinghal05 neerajsinghal05 force-pushed the Multi_upload_Policy branch 2 times, most recently from dc1d1f6 to 9ae52cd Compare November 14, 2023 12:33
Copy link
Collaborator

@rahuldash171 rahuldash171 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved .

@rahuldash171 rahuldash171 merged commit 44f61d8 into develop Nov 28, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants