[Proposal] RemoveObject & RemoveObjects to return more info about successful deletion

[vadmeste]

Currently RemoveObject() and RemoveObjects() only return errors for failed deletions.

However, S3 spec returns two information with successful deletion:
x-amz-version-id
x-amz-delete-marker

which could be useful to have them.

For example, mc wants to show a delete marker is added in a versioning bucket instead of removed message.

[harshavardhana]

@vadmeste is this fixed?

[vadmeste]

@vadmeste is this fixed?

No, this is not fixed and this needs a major version bump since RemoveObject() needs to return another additional result (or we add a new API ?)

[harshavardhana]

@vadmeste is this fixed?

No, this is not fixed and this needs a major version bump since RemoveObject() needs to return another additional result (or we add a new API ?)

We can do a version bump since there is another PR that is attempting to do version bump as well

[vadmeste]

API changes for RemoveObjects:

Before:

func (c Client) RemoveObjects(ctx context.Context, bucketName string, objectsCh <-chan ObjectInfo, opts RemoveObjectsOptions) <-chan RemoveObjectError {

After:

func (c Client) RemoveObjects(ctx context.Context, bucketName string, objectsCh <-chan ObjectInfo, opts RemoveObjectsOptions) <-chan RemoveObjectResult {

Before we were only returning errors, now we return successful deletion as well in the returned channel, for that RemoveObjectError is renamed to RemoveObjectResult.

type RemoveObjectResult struct {
        ObjectName      string
        ObjectVersionID string

        DeleteMarker          bool
        DeleteMarkerVersionID string

        Err error
}

I added DeleteMarker & DeleteMarkerVersionID in RemoveObjectResult{}, and I renamed VersionID to ObjectVersionID

API changes for RemoveObject:

Before:

func (c Client) RemoveObject(ctx context.Context, bucketName, objectName string, opts RemoveObjectOptions) error {

After:

func (c Client) RemoveObject(ctx context.Context, bucketName, objectName string, opts RemoveObjectOptions) RemoveObjectResult {

[harshavardhana]

I think we should deprecate this API, we only need RemoveObjects() AFAICS.

func (c Client) RemoveObject(ctx context.Context, bucketName, objectName string, opts RemoveObjectOptions) RemoveObjectResult {

[vadmeste]

I think we should deprecate this API, we only need RemoveObjects() AFAICS.

yeah we can do that, but the code will be little "ugly" for users who want to delete only one object:

        objectsCh := make(chan minio.ObjectInfo, 1)
        objectsCh <- minio.ObjectInfo{Key: "testobject"}
        close(objectsCh)
        resultCh := s3Client.RemoveObjects(context.Background(), "my-bucketname", objectsCh, minio.RemoveObjectsOptions{})
        for r := range resultCh {
                if r.Err != nil {
                        log.Fatalln("Failed to remove " + r.ObjectName + ", error: " + r.Err.Error())
                }
        }

[klauspost]

Yeah, a single delete option is much nicer API-wise.

[vadmeste]

@harshavardhana maybe we can avoid deprecating and keep the API as it is for now. Any user who wants more information about delete-marker & delete-marker-version-id will have to use DeleteObjects()

[harshavardhana]

yeah we can do that, but the code will be little "ugly" for users who want to delete only one object:

It's very rare that users only have to delete one object and we are providing a proper streaming approach. It is not a big deal to use only RemoveObjects() API

@harshavardhana maybe we can avoid deprecating and keep the API as it is for now. Any user who wants more information about delete-marker & delete-marker-version-id will have to use DeleteObjects()

We can't add two different APIs unfortunately.

[vadmeste]

We can't add two different APIs unfortunately.

Yes, I am not adding any new API. What I am saying is, we keep DeleteObject() as it is but not deprecate it and we break DeleteObjects() to return delete marker information

[harshavardhana]

Yes, I am not adding any new API. What I am saying is, we keep DeleteObject() as it is but not deprecate it and we break DeleteObjects() to return delete marker information

No, if we are breaking just remove older ones we don't need a single deleteObject() - for that we can use core.DeleteObject() - higher level APIs are meant for more mc like tools.

[vadmeste]

What about removing RemoveObject but simplifying removing one object using a new API, so it will look like this:

 resultCh := s3Client.RemoveObjects(
         context.Background(), 
         "my-bucketname", 
          minio.GenObjectsList([]minio.ObjectInfo{Key: "testobject"}), 
          minio.RemoveObjectsOptions{},
)

[andreykeen]

API changes for RemoveObjects:

Before:

func (c Client) RemoveObjects(ctx context.Context, bucketName string, objectsCh <-chan ObjectInfo, opts RemoveObjectsOptions) <-chan RemoveObjectError {

After:

func (c Client) RemoveObjects(ctx context.Context, bucketName string, objectsCh <-chan ObjectInfo, opts RemoveObjectsOptions) <-chan RemoveObjectResult {

Before we were only returning errors, now we return successful deletion as well in the returned channel, for that RemoveObjectError is renamed to RemoveObjectResult.

type RemoveObjectResult struct {
        ObjectName      string
        ObjectVersionID string

        DeleteMarker          bool
        DeleteMarkerVersionID string

        Err error
}

I added DeleteMarker & DeleteMarkerVersionID in RemoveObjectResult{}, and I renamed VersionID to ObjectVersionID

API changes for RemoveObject:

Before:

func (c Client) RemoveObject(ctx context.Context, bucketName, objectName string, opts RemoveObjectOptions) error {

After:

func (c Client) RemoveObject(ctx context.Context, bucketName, objectName string, opts RemoveObjectOptions) RemoveObjectResult {

Are you planning to release a new version with these changes in the near future?

[harshavardhana]

This enhancement is not needed refer PR #1456