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

add arg builder to set/get objectRetention API #938

Merged
merged 5 commits into from May 27, 2020

Conversation

sinhaashish
Copy link
Contributor

@sinhaashish sinhaashish commented May 15, 2020

Added some functional Test to set the object retention with governance mode and then set the lock with empty params so that object can be removed.
The test case will will until PR minio/minio#9677 is merged and reflected in play.

api/src/main/java/io/minio/MinioClient.java Outdated Show resolved Hide resolved
api/src/main/java/io/minio/MinioClient.java Outdated Show resolved Hide resolved
api/src/main/java/io/minio/MinioClient.java Outdated Show resolved Hide resolved
api/src/main/java/io/minio/MinioClient.java Show resolved Hide resolved
api/src/main/java/io/minio/MinioClient.java Outdated Show resolved Hide resolved
api/src/main/java/io/minio/SetObjectRetentionArgs.java Outdated Show resolved Hide resolved
docs/API.md Outdated Show resolved Hide resolved
docs/API.md Outdated Show resolved Hide resolved
throws IOException, NoSuchAlgorithmException, InvalidKeyException, InvalidResponseException,
InsufficientDataException, InternalException, ErrorResponseException,
InvalidBucketNameException, InvalidPortException, InvalidEndpointException,
RegionConflictException, IllegalArgumentException {
Copy link
Member

Choose a reason for hiding this comment

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

Shorten exception list by catching MinioException.

examples/GetObjectRetentionConfig.java Outdated Show resolved Hide resolved
@balamurugana
Copy link
Member

I would recommend to have short and precise commit message to patches (including git log and github PR title/description).

@sinhaashish sinhaashish changed the title Set/Get Object Retention [WIP]Set/Get Object Retention May 19, 2020
@sinhaashish sinhaashish changed the title [WIP]Set/Get Object Retention Set/Get Object Retention May 22, 2020
api/src/main/java/io/minio/MinioClient.java Outdated Show resolved Hide resolved
api/src/main/java/io/minio/MinioClient.java Outdated Show resolved Hide resolved
api/src/main/java/io/minio/SetObjectRetentionArgs.java Outdated Show resolved Hide resolved
docs/API.md Outdated Show resolved Hide resolved
examples/GetObjectRetentionConfig.java Outdated Show resolved Hide resolved
examples/SetObjectRetentionConfig.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

LGTM. Please look at the build failures

docs/API.md Outdated Show resolved Hide resolved
examples/SetObjectRetention.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
// they were unprotected if you have the s3:bypassGovernanceMode permission. These
// operations include deleting an object version, shortening the retention period, or
// removing the Object Lock by placing a new lock with empty parameters.
ZonedDateTime shortenedRetentionUntil = ZonedDateTime.now().plusDays(1);
Copy link
Member

Choose a reason for hiding this comment

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

  1. I think you are testing bypassGovernanceMode() here. If so it needs to be merged in above.
  2. I guess bypassGovernanceMode allow to change Retention where RetentionUntil time could be higher or lower. If this is the test you want to do, you could have the test doing both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is to set retention with a shortened retention period. As per s3 , retention period can shortened only by setting bypassGovernance to true.
You can perform operations on object versions that are locked in governance mode as if they were unprotected if you have the s3:BypassGovernanceRetention permission. These operations include deleting an object version, shortening the retention period, or removing the Object Lock by placing a new lock with empty parameters.

bypassGovernanceMode is tested in all the related functional test test as i set the empty lock by setting bypassGovernance to true and then i am able to delete the object.

@sinhaashish sinhaashish changed the title Set/Get Object Retention [WIP]Set/Get Object Retention May 27, 2020
@sinhaashish sinhaashish changed the title [WIP]Set/Get Object Retention Set/Get Object Retention May 27, 2020
kannappanr
kannappanr previously approved these changes May 27, 2020
Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

LGTM

functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
functional/FunctionalTest.java Outdated Show resolved Hide resolved
@balamurugana balamurugana changed the title Set/Get Object Retention add arg builder to set/get objectRetention API May 27, 2020
Copy link
Member

@balamurugana balamurugana left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kannappanr kannappanr left a comment

Choose a reason for hiding this comment

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

LGTM

@kannappanr kannappanr merged commit 73ad509 into minio:master May 27, 2020
@sinhaashish sinhaashish deleted the object-retention branch May 27, 2020 19:09
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.

None yet

3 participants