-
Notifications
You must be signed in to change notification settings - Fork 1
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
Support S3 Objects List ETag map #126
Conversation
@@ -157,6 +158,114 @@ func (a *DefaultAwsOps) BucketObjects(bucket, s3Prefix string, concurrency int, | |||
} | |||
} | |||
|
|||
func (a *DefaultAwsOps) BucketObjectsMap(bucket, s3Prefix string, concurrency int, recursive bool, reg *regexp.Regexp) (map[string]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to review this whole method in detail. I'll annotate what changed, apart from the signature.
return | ||
} | ||
|
||
bm := getObjectsAllMap(resp, s3Prefix, reg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only line that differs materially from BucketObjects
. It calls the new getObjectsAllMap
method instead of getObjectsAll
.
binaryMeta[strings.TrimPrefix(s[1], s3Prefix)] = *key.ETag | ||
} | ||
} else { | ||
binaryMeta[strings.TrimPrefix(*key.Key, s3Prefix)] = *key.ETag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is nearly identical to the existing getObjectsAll
method except that it returns a map and sets the map key/value pairs here instead of simply appending to a string array.
@@ -19,6 +19,7 @@ import ( | |||
type AwsOps interface { | |||
BucketDirs(bucket, s3Prefix string) ([]string, error) | |||
BucketObjects(bucket, s3Prefix string, concurrency int, recursive bool, reg *regexp.Regexp) ([]string, error) | |||
BucketObjectsMap(bucket, s3Prefix string, concurrency int, recursive bool, reg *regexp.Regexp) (map[string]string, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I'd just suggest making this more obvious that it's a map of ETag values. That could be a comment, but maybe renaming the method works better. Or if it's possible we'd need more metadata in the future, a map of structs that might someday contain LastModified time or whatever.
Adds a
BucketObjectsMap
method that returns a map of S3 keys to ETag values. This method is nearly identical to the existingBucketObjects
method, but differs slightly to return a map with the ETag values.