Add API to compose objects through server-side copying #715

donatello · 2017-06-19T21:29:11Z

The new ComposeObject API provides a way to create objects by
concatenating existing objects. It takes a list of source objects
along with optional start-end range specifications, and concatenates
them into a new object.

The API supports:

Create an object from upto 10000 existing objects.
Create object of any size upto 5TiB.
Creates objects from even existing objects that are larger than
5GiB (part size limit in multipart PUTs)
Support copy-conditions on each source object separately.
Support SSE-C (i.e. Server-Side-Encryption with Customer provided
key) for both encryption of destination object, and decryption of
source objects.
Support for setting/replacing custom metadata in the destination
object.

This API has been used to refactor the CopyObject API - that API now
supports source objects of any size, SSE-C for source and destination,
and settings custom metadata.

Finally, the type for a source object (SourceInfo) allows the user to set custom headers (by hand) for further extended usage (such as using AWS KMS based SSE).

houndci-bot · 2017-06-19T21:29:20Z

api-compose-object.go

+	customMetadata map[string]string
+}
+
+// Function to create a DesinationInfo instance - encryptionInfo may


comment on exported function NewDestinationInfo should be of the form "NewDestinationInfo ..."

houndci-bot · 2017-06-19T21:29:20Z

api-compose-object.go

+	"github.com/minio/minio-go/pkg/s3utils"
+)
+
+// A type representing Server-Side-Encryption parameters specified by


comment on exported type SSEInfo should be of the form "SSEInfo ..." (with optional leading article)

donatello · 2017-06-20T02:28:52Z

Note to reviewers: This PR breaks compatibility by changing the CopyObject API signature. It was deemed necessary to bring an improved version in.

krisis

Overall the PR looks good to me. There are a few comments that need to be addressed before we merge.

krisis · 2017-06-21T10:47:31Z

api-compose-object.go

+			return httpRespToErrorResponse(resp, dst.bucket, dst.object)
+		}
+		// Decode copy response on success.
+		cpObjRes := copyObjectResult{}


The following snippet of code only confirms that on copy object success the XML response parses fine into a valid copyObjectResult{}. If that is not important, we can skip reading the response body and decoding it.

You're right, it is not required here.

krisis · 2017-06-21T10:54:53Z

docs/API.md

@@ -61,6 +61,7 @@ func main() {
 |[`ListIncompleteUploads`](#ListIncompleteUploads) | [`RemoveIncompleteUpload`](#RemoveIncompleteUpload) |  |  |  [`ListenBucketNotification`](#ListenBucketNotification)  |
 |   | [`FPutObject`](#FPutObject)  | |   |   |
 |   | [`FGetObject`](#FGetObject)  | |   |   |
+|   | [`ComposeObject`](#FGetObject)  | |   |   |


s/(#FGetObject)/(#ComposeObject)/

krisis · 2017-06-21T10:57:28Z

docs/API.md

@@ -559,6 +560,58 @@ if err != nil {
 }
 ```

+<a name="ComposeObject"></a>


Documentation on CopyObject needs to be updated too.

krisis · 2017-06-21T10:58:33Z

docs/API.md

@@ -559,6 +560,58 @@ if err != nil {
 }
 ```

+<a name="ComposeObject"></a>
+### ComposeObject(dst DestinationInfo, srcs []SourceInfo) error


We need to expand on DestinationInfo and SourceInfo types as well.

NewSSEInfo, NewSourceInfo and NewDestinationInfo are added in docs. The types are only documented via godoc for now, in keeping with the existing style (e.g. CopyCondition was documented only via godoc). We may add documentation for types in the API.md later but that is a matter for discussion.

krisis · 2017-06-21T10:59:04Z

examples/s3/composeobject.go

+
+	// Prepare source decryption key (here we assume same key to
+	// decrypt all source objects.)
+	decKey := NewSSEInfo([]byte{1, 2, 3}, "")


shouldn't this be minio.NewSSEInfo?

krisis · 2017-06-21T10:59:31Z

docs/API.md

+	srcs := []minio.SourceInfo{src1, src2, src3}
+
+	// Prepare destination encryption key
+	encKey := NewSSEInfo([]byte{8, 9, 0}, "")


Shouldn't this be minio.NewSSEInfo?

krisis · 2017-06-21T11:00:36Z

docs/API.md

+```go
+	// Prepare source decryption key (here we assume same key to
+	// decrypt all source objects.)
+	decKey := NewSSEInfo([]byte{1, 2, 3}, "")


Shouldn't this be minio.NewSSEInfo?

krisis · 2017-06-21T11:00:57Z

examples/s3/composeobject.go

+	srcs := []minio.SourceInfo{src1, src2, src3}
+
+	// Prepare destination encryption key
+	encKey := NewSSEInfo([]byte{8, 9, 0}, "")


Shouldn't this be minio.NewSSEInfo?

krisis · 2017-06-21T11:01:40Z

examples/s3/copyobject.go

@@ -42,24 +42,28 @@ func main() {
 	// Enable trace.
 	// s3Client.TraceOn(os.Stderr)

+	// Source object
+	src := NewSourceInfo("my-sourcebucketname", "my-sourceobjectname", nil)


Shouldn't this be minio.NewSourceInfo?

krisis · 2017-06-21T11:01:57Z

examples/s3/copyobject.go

+	// src.SetMatchETagExceptCond("31624deb84149d2f8ef9c385918b653a")
+
+	// Destination object
+	dst := NewDestinationInfo("my-bucketname", "my-objectname", nil)


Shouldn't this be minio.NewDestinationInfo?

donatello · 2017-06-22T00:04:17Z

This is ready for review.

harshavardhana · 2017-06-22T00:13:39Z

api-compose-object.go

+}
+
+// NewSSEInfo - specifies (binary or un-encoded) encryption key and
+// algorithm name. If algo is empty, it defaults to "AES256".


Perhaps mention the AES256 AWS S3 link.

harshavardhana · 2017-06-22T00:13:59Z

api-compose-object.go

+
+// internal method to output the generated SSE headers.
+func (e *SSEInfo) getSSEHeaders(isCopySource bool) map[string]string {
+	if e == nil {


Is this if e == nil necessary? - perhaps okay to let the function fail?

No it is needed here. When getSSEHeaders is called, if there is no key provided during initialization, this safely returns. The other option is to check if the key is nil before calling this function, but I found that to be more repetitive.

i see.. should we ignore it outside rather than letting this call succeed?

If we checked if key is nil outside, then each place of using this function would look like (for e.g.):

if src.key != nil { for k, v := src.key.getSSEHeaders(false) { // do stuff. } }

The outer if block would be repeated in each usage of this function. In the current implementation it is neatly tucked away inside getSSEHeaders and it returns a nil back which can be safely used in a for-range loop.

Correct but its quite an unnatural usage, usually unallocated struct means its a bug of sort and we are sort of hiding it. That is my only concern.

harshavardhana · 2017-06-22T00:14:54Z

api-compose-object.go

+// `REPLACE`, so that metadata headers from the source are not copied
+// over.
+func (d *DestinationInfo) getCustomHeadersMap(withCopyDirectiveHeader bool) map[string]string {
+	if d == nil || d.customMetadata == nil {


Same as above..

Again same reasoning as before - this is less repetitive.

harshavardhana · 2017-06-22T00:16:14Z

api-compose-object.go

+	if isCopySource {
+		cs = "copy-source-"
+	}
+	hsum := md5.Sum(e.key)


You could use sumMD5() ?

harshavardhana · 2017-06-22T00:16:49Z

api-compose-object.go

+
+	start, end int64
+
+	// collection of headers to send with every upload-part-copy


// collect of request headers sent with every upload-part-copy

Updated to:

// Headers to send with the upload-part-copy request involving // this source object.

The intension is to allow users to set custom headers on their own in case they need some feature not encapsulated by this library.

harshavardhana · 2017-06-22T00:17:23Z

api-compose-object.go

+	}
+
+	// Set the source header
+	r.Headers.Set("x-amz-copy-source", s3utils.EncodePath(fmt.Sprintf("/%s/%s", bucket, object)))


Perhaps use path.Join() ? it is okay to not have preceding /

Ok, just used + here.

harshavardhana · 2017-06-22T00:20:35Z

constants.go

 // putObject behaves internally as multipart.
 const minPartSize = 1024 * 1024 * 64

+// copyPartSize - default (and maximum) part size to copy in a
+// copy-object request.
+const copyPartSize = 1024 * 1024 * 1024 * 5


why not 64MB? no reason for this to be 5MB?

This number is actually 5 GiB. I will update the comment to make this more apparent.

Since this is only server-side copying, the most efficient thing to do is to reduce the number of network requests - this is done by copying the largest possible object-part in each request.

Actually let's not do that.. 5GiB causes for erasure coding block to become super big on a 4 disk setup. Say for example 5GiB *2 so each disk shard will be 2.5GiB. We should avoid this worst case scenario since this would cause easily 2-3secsec worth of delay for ReadFileWithVerify().

We should pick 525MB which would make it 10,000 parts to upload maximum 5TB object. In which case each shard will be at max 256MB.

donatello · 2017-06-22T01:53:18Z

Review comments addressed!

harshavardhana · 2017-06-22T03:03:38Z

api-compose-object.go

+		var currOffset, partSize int64
+		for currOffset < size {
+			if currOffset+copyPartSize < size {
+				partSize = copyPartSize


Just use

// Calculate the optimal parts info for a given size. totalPartsCount, partSize, lastPartSize, err := optimalPartInfo(fileSize) if err != nil { return 0, err }

Since you know the right file size it is better to know the optimal part size. 5GiB is not an ideal size here.

I understand your point about the bad case for erasure coding but there is another problem. If we use up more parts for the first few sources, then there may be too many parts for later sources in the input and it may exceed the max parts limit. I will think a bit and figure out a working algo.

balamurugana · 2017-06-22T12:56:41Z

examples/s3/copyobject.go


 	// Initiate copy object.
-	err = s3Client.CopyObject("my-bucketname", "my-objectname", "/my-sourcebucketname/my-sourceobjectname", copyConds)
+	err = s3Client.CopyObject(dst, src)


If source object was uploaded with client side encryption and respective headers saved, does CopyObject() take care of those headers as well.

balamurugana · 2017-06-22T12:57:49Z

examples/s3/composeobject.go

+	src3.SetMatchETag("5918b653a31624deb84149d2f8ef9c38")
+
+	// Create slice of sources.
+	srcs := []minio.SourceInfo{src1, src2, src3}


If source objects were uploaded with client side encryption, what is the expected behavior of ComposeObject()?

Encrypted sources must be decrypted for copying - see https://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadUploadPartCopy.html - relevant quote:

If the source object is encrypted using server-side encryption with a customer-provided encryption key, you must use the following headers providing encryption information so that Amazon S3 can decrypt the object for copying.

So ComposeObject also does the same decrypt and copy - each encrypted source must have keys specified for decryption. (Note that this server-side-encryption not client side)

I understand server side encryption. The question is asked for client side encryption specifically.

Oops, sorry I misread. In the case of client side encryption, the encrypted content would be just copied transparently by the server (it is not aware of client-side encrypted content) - most likely concatenating (client-side-)encrypted objects arbitrarily will lead to garbled data, but server cannot detect this - it is a user error.

Is it possible to find and error out in this case? I see minio-java sets below header.

headerMap.put("x-amz-meta-x-amz-key", encDataKey); headerMap.put("x-amz-meta-x-amz-iv", ivString); headerMap.put("x-amz-meta-x-amz-matdesc", "{}");

No, I do not think if we can do this correctly - we cannot try and interpret custom metadata set by the user or other tools that the user uses.

As per http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html, client side encryption uses x-amz-meta-x-amz-key header. If client side encryption is used for any of source objects, we should error out than performing compose object. The current behavior leads to data corruption if source objects are deleted after compose.

does this comment make sense?

I am not sure if it is appropriate to do this - as the encryption scheme is a client side scheme. Copying encrypted objects may be a valid operation in certain situations (client logic may be something complicated and it may be an intended operation - there is no need for the library to stop this). It needs further discussion - and a call can be taken after releasing this.

If only one object will be copied (e.g. via copyobject, with no explicit user metadata headers passed in), this problem is mitigated as the headers from source will be copied to the destination.

As this issue discussed with @abperiasamy, issue #738 is opened and this needs to be addressed after this PR merged.

balamurugana · 2017-06-22T12:59:01Z

constants.go

 // putObject behaves internally as multipart.
 const minPartSize = 1024 * 1024 * 64

+// copyPartSize - default (and maximum) part size to copy in a
+// copy-object request (5GiB)
+const copyPartSize = 1024 * 1024 * 1024 * 5


If humanize package is already in use, you could use that.

Using that lib for parsing requires checking for errors. It does not add much.

I was talking about using const copyPartSize = 5 * humanize.GiByte

balamurugana · 2017-06-22T13:09:57Z

api-put-object-copy.go

-	// Return nil on success.
-	return nil
+// CopyObject - copy a source object into a new object
+func (c Client) CopyObject(dst DestinationInfo, src SourceInfo) error {


Why is this function made to break backward compatibility?

does this comment make sense?

Without breaking backward compatibility, we would have a very complex function(s) to support copy-object with support for decryption of source, encryption of destination, multiple copy-conditions, sub-range of source and setting user metadata headers. Since this is a significant improvement to the existing API, it was agreed to break compatibility and make a new release.

ok. got it.

balamurugana · 2017-06-22T13:12:51Z

api-compose-object.go

+}
+
+// internal method to output the generated SSE headers.
+func (e *SSEInfo) getSSEHeaders(isCopySource bool) map[string]string {


why e is short form to SSEInfo? I think you can name it better

balamurugana · 2017-06-22T13:16:21Z

api-compose-object.go

+// DestinationInfo - type with information about the object to be
+// created via server-side copy requests, using the Compose API.
+type DestinationInfo struct {
+	bucket, object string


I think bucket and object can be listed in two lines for better readability.

balamurugana · 2017-06-22T13:22:26Z

api-compose-object.go

+	}
+	for k, v := range d.customMetadata {
+		if !strings.HasPrefix(k, "x-amz-meta-") {
+			k = "x-amz-meta-" + k


is it required key should start with x-amz-meta-. if so, you could error out in new(). If two keys named x-amz-meta-my-key and my-key one key/value is lost.

This is handled in NewDestinationInfo() validation now.

balamurugana · 2017-06-22T13:22:50Z

api-compose-object.go

+type SourceInfo struct {
+	bucket, object string
+
+	start, end int64


same as above

vadmeste · 2017-06-22T14:36:38Z

api-compose-object.go

+	if withCopyDirectiveHeader {
+		r["x-amz-metadata-directive"] = "REPLACE"
+	}
+	for k, v := range d.customMetadata {


Can you add one comment to this for ?

vadmeste · 2017-06-22T14:44:28Z

api-compose-object.go

+		return err
+	}
+	if len(srcs) > maxPartsCount {
+		return ErrInvalidArgument("Cannot copy more than 10000 source objects.")


Can you replace 10000 with maxPartsCount ?

vadmeste · 2017-06-22T14:45:28Z

api-compose-object.go

+	if err := s3utils.CheckValidObjectName(dst.object); err != nil {
+		return err
+	}
+	if len(srcs) > maxPartsCount {


Can you also return an error when srcs is empty ? this function is little long, returning early error will be easier to read.

vadmeste · 2017-06-22T14:59:14Z

api-compose-object.go

+	r := SourceInfo{
+		bucket:  bucket,
+		object:  object,
+		start:   -1,


I know start = -1 means start is unspecified but we can also set it to zero by default to simplify the code. Do we really need -1 state ?

Before the size of the source is known, 0 <= start <= end represents a valid range specification so we need to use start = -1 to represent an unspecified range. I have added lots of comments to make this clearer now. Ref: see doc for x-amz-copy-source-range at https://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadUploadPartCopy.html

houndci-bot · 2017-06-22T18:59:49Z

api-compose-object.go

+// `x-amz-meta-` if they are not prefixed in the input. If `metadata`
+// is not specified, and only one source
+
+// encryptionInfo may be nil to indicate that no-encryption is to be


comment on exported function NewDestinationInfo should be of the form "NewDestinationInfo ..."

houndci-bot · 2017-06-22T18:59:49Z

api-compose-object.go

+// `x-amz-meta-` if they are not prefixed in the input. If `metadata`
+// is not specified, and only one source
+
+// encryptionInfo may be nil to indicate that no-encryption is to be


comment on exported function NewDestinationInfo should be of the form "NewDestinationInfo ..."

balamurugana · 2017-06-23T02:29:00Z

examples/s3/composeobject.go

+	src3.SetMatchETag("5918b653a31624deb84149d2f8ef9c38")
+
+	// Create slice of sources.
+	srcs := []minio.SourceInfo{src1, src2, src3}


As per http://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html, client side encryption uses x-amz-meta-x-amz-key header. If client side encryption is used for any of source objects, we should error out than performing compose object. The current behavior leads to data corruption if source objects are deleted after compose.

harshavardhana · 2017-06-26T16:48:53Z

ping @krisis @balamurugana do you guys have more reviews on this PR? @donatello are you going to send an update?

houndci-bot · 2017-06-27T01:21:22Z

api-compose-object.go

+
+// partsRequired is ceiling(size / copyPartSize)
+func partsRequired(size int64) int {
+	var r int64 = size / copyPartSize


should omit type int64 from declaration of var r; it will be inferred from the right-hand side

This is not needed you can make this int64(size/copyPartSize)

donatello · 2017-06-27T01:49:45Z

This PR is updated again.

The previous implementation had a bug.

Example case of bug: two sources of size s given for concatenating, where s = 5GiB+1. Then compose object would try to upload 4 parts of sizes 5GiB, 1, 5GiB, 1. This fails because one of the parts not in the end is of length < 5MiB. In the fix, we make every part of a particular source (almost) the same size. So for the above example, it would generate 4 parts sized: 2.5GiB+1, 2.5GiB, 2.5GiB+1, 2.5GiB.

donatello · 2017-06-27T02:23:15Z

That's fixed in the latest change.

…

On Jun 26, 2017 7:08 PM, "Harshavardhana" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In api-compose-object.go <#715 (comment)>: > + partIndex++ + } + } + + // 3. Make final complete-multipart request. + _, err = c.completeMultipartUpload(dst.bucket, dst.object, uploadID, + completeMultipartUpload{Parts: objParts}) + if err != nil { + err = fmt.Errorf("Error in complete-multipart request - %v", err) + } + return err +} + +// partsRequired is ceiling(size / copyPartSize) +func partsRequired(size int64) int { + var r int64 = size / copyPartSize This is not needed you can make this int64(size/copyPartSize) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#715 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMNe50Fv4aDvu55jN8kfrOZ0roZ-QZgks5sIGQAgaJpZM4N-0IV> .

tejaycar · 2017-06-29T14:55:05Z

@harshavardhana Are there really outstanding changes still? It looks like github is blocking merge, but the only "requested change" I can see is also marked as outdated...

harshavardhana · 2017-06-29T16:57:08Z

@harshavardhana Are there really outstanding changes still? It looks like github is blocking merge, but the only "requested change" I can see is also marked as outdated...

@tejaycar its not was waiting on the other reviewers.

harshavardhana · 2017-06-29T16:57:52Z

@balamurugana did you get a chance to look at the latest changes?

tejaycar · 2017-06-29T16:59:36Z

Ah, that's great news. Are you planning to cut a new release when this is merged? We're really hoping to get it pulled into the next release of Mattermost, but would prefer to do that from a formal release rather than master.

harshavardhana · 2017-06-29T17:00:08Z

Ah, that's great news. Are you planning to cut a new release when this is merged? We're really hoping to get it pulled into the next release of Mattermost, but would prefer to do that from a formal release rather than master.

Yes @tejaycar it will be a major bump since we broke existing API.

tejaycar · 2017-06-29T17:01:07Z

Sweet! You guys rock, I'm looking forward to it.

The new ComposeObject API provides a way to create objects by concatenating existing objects. It takes a list of source objects along with optional start-end range specifications, and concatenates them into a new object. The API supports: * Create an object from upto 10000 existing objects. * Create objects upto 5TiB in size, from source objects of any size. * Support copy-conditions on each source object separately. * Support SSE-C (i.e. Server-Side-Encryption with Customer provided key) for both encryption of destination object, and decryption of source objects. * Support for setting/replacing custom metadata in the destination object. This API has been used to refactor the CopyObject API - that API now supports source objects of any size, SSE-C for source and destination, and settings custom metadata.

donatello · 2017-07-01T02:35:37Z

More tests are added. I believe the PR is complete now and ready for merge.

balamurugana

With one exception Multiple sources those use client side encryption in copy object api should return error. #738
Not tested

houndci-bot reviewed Jun 19, 2017

View reviewed changes

donatello force-pushed the compose-object branch 4 times, most recently from 9d32a57 to 1a4a0dc Compare June 20, 2017 02:23

donatello changed the title ~~WIP: Add API to compose objects through server-side copying~~ Add API to compose objects through server-side copying Jun 20, 2017

donatello mentioned this pull request Jun 20, 2017

Add large-object support for CopyObject API #644

Closed

donatello force-pushed the compose-object branch from 1a4a0dc to 13a2393 Compare June 20, 2017 17:26

deekoder requested review from vadmeste and krisis June 20, 2017 19:24

donatello force-pushed the compose-object branch 3 times, most recently from 25cc5e2 to daa0d07 Compare June 21, 2017 00:55

krisis requested changes Jun 21, 2017

View reviewed changes

tejaycar mentioned this pull request Jun 21, 2017

CopyObject Does not support metadata #690

Closed

donatello force-pushed the compose-object branch 3 times, most recently from 2c19576 to 249d6a2 Compare June 21, 2017 23:59

harshavardhana requested changes Jun 22, 2017

View reviewed changes

donatello force-pushed the compose-object branch from 249d6a2 to 0d646e6 Compare June 22, 2017 01:46

donatello force-pushed the compose-object branch from 0d646e6 to 1c4ea3b Compare June 22, 2017 01:58

harshavardhana requested changes Jun 22, 2017

View reviewed changes

balamurugana reviewed Jun 22, 2017

View reviewed changes

vadmeste reviewed Jun 22, 2017

View reviewed changes

donatello force-pushed the compose-object branch from 1c4ea3b to b8fe0a8 Compare June 22, 2017 18:59

houndci-bot reviewed Jun 22, 2017

View reviewed changes

donatello force-pushed the compose-object branch 3 times, most recently from ef81f52 to 8c55995 Compare June 22, 2017 23:12

balamurugana requested changes Jun 23, 2017

View reviewed changes

donatello force-pushed the compose-object branch from 8c55995 to 652c95a Compare June 27, 2017 01:21

houndci-bot reviewed Jun 27, 2017

View reviewed changes

donatello force-pushed the compose-object branch 2 times, most recently from e001a04 to 6316b75 Compare June 27, 2017 01:45

krisis approved these changes Jun 29, 2017

View reviewed changes

donatello force-pushed the compose-object branch from 6316b75 to e773fe7 Compare June 30, 2017 23:30

donatello force-pushed the compose-object branch from e773fe7 to 73716f3 Compare July 1, 2017 01:38

harshavardhana approved these changes Jul 1, 2017

View reviewed changes

balamurugana mentioned this pull request Jul 2, 2017

Multiple sources those use client side encryption in copy object api should return error. #738

Closed

balamurugana approved these changes Jul 2, 2017

View reviewed changes

harshavardhana merged commit 129fe89 into minio:master Jul 2, 2017

poornas added a commit to poornas/minio-go that referenced this pull request Jul 3, 2017

fold in compose objects functional_tests from minio#740 PR and minio#715

e1a745b


		start, end int64

		// collection of headers to send with every upload-part-copy

Add API to compose objects through server-side copying #715

Add API to compose objects through server-side copying #715

Conversation

donatello commented Jun 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donatello commented Jun 20, 2017

krisis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donatello commented Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donatello Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donatello Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donatello commented Jun 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balamurugana Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshavardhana commented Jun 26, 2017

Choose a reason for hiding this comment

donatello commented Jun 22, 2017 •

edited

Loading

donatello Jun 22, 2017 •

edited

Loading

donatello Jun 22, 2017 •

edited

Loading

balamurugana Jun 27, 2017 •

edited

Loading