Adding swagger spec for RecoveryServices.SiteRecovery service. #1209

avneeshrai · 2017-05-09T18:14:50Z

Adding swagger spec for RecoveryServices.SiteRecovery service (2016-08-10).

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

This change is

…8-10).

msftclas · 2017-05-09T18:14:54Z

@avneeshrai,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

dsgouda

Is there a reason why this is not being added to the directory here instead

avneeshrai · 2017-05-09T19:20:54Z

Note for the reviewers:
This has already been reviewed in PR #1016
I feel @veronicagg would be the best to review this PR as she already has context on it.

Validator tool shows following category of issues:
"code": "OperationsAPIImplementationValidation",
"code": "ProvidersPathValidation",
"code": "PutRequestResponseValidation",
"code": "TrackedResourceListByImmediateParent",
"code": "XmsExamplesProvidedValidation",
"code": "DeleteOperationNameValidation",
"code": "DefinitionsPropertiesNamesCamelCase",
"code": "PatchBodyParametersSchemaValidation",
"code": "BodyTopLevelProperties",
"code": "AvoidNestedProperties",
"code": "BooleanPropertyNotRecommended"

Except on "code": "OperationsAPIImplementationValidation" we had closed on all other issues in the current thread. About the implementation of operations API, we expect operations API for our provider Microsoft.RecoveryServices to be present in https://github.com/Azure/azure-rest-api-specs/blob/08f0f317ccb89b86f552232bd8fce02f98889463/arm-recoveryservices/2016-06-01/swagger/vaultusages.json spec (which is already there). Let me know if we have a reason to duplicate it here.
2. Examples for 102 out of 109 APIs have been included in the PR. We couldn’t include examples for remaining 7 because of the issues mentioned below with the validator tool:

a)All Job related APIs
"ReplicationJobs_Get",
"ReplicationJobs_Resume",
"ReplicationJobs_GetTrackAsyncOperation",
"ReplicationJobs_Restart",
"ReplicationJobs_Cancel",
"ReplicationJobs_Export",
"ReplicationJobs_List"
Reason: Recursive call in ASRTask model

Error seen in validator tool:
error: Found errors in validating the response with statusCode "200" for x-ms-example "Gets the job details." in operation "ReplicationJobs_Get".:

{ code: 'RESPONSE_VALIDATION_ERROR',
message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Gets the job details." in operation "ReplicationJobs_Get".',
innerErrors:
[ { code: 'INVALID_RESPONSE_BODY',
errors:
[ { code: undefined,
message: 'Maximum call stack size exceeded',
path: undefined } ],
message: 'Invalid body: Maximum call stack size exceeded',
path: [] } ] }

b)Many APIs
E.g.: ReplicationFabrics_Get

Fields having “null” value are not allowed.

Error seen in validator tool:
error: Found errors in validating the response with statusCode "200" for x-ms-example "Gets the details of a Site." in operation "ReplicationFabrics_Get".:

{ code: 'RESPONSE_VALIDATION_ERROR',
message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Gets the details of a Site." in operation "ReplicationFabrics_Get".',
innerErrors:
[ { code: 'INVALID_RESPONSE_BODY',
errors:
[ { code: 'INVALID_TYPE',
params: [ 'string', 'null' ],
message: 'Expected type string but found type null',
path:
[ 'properties',
'rolloverEncryptionDetails',
'kekCertThumbprint' ],
description: 'Gets or sets the key encryption key certificate thumbprint.' },
{ code: 'INVALID_TYPE',
params: [ 'string', 'null' ],
message: 'Expected type string but found type null',
path: [ 'properties', 'encryptionDetails', 'kekCertThumbprint' ],
description: 'Gets or sets the key encryption key certificate thumbprint.' } ],
message: 'Invalid body: Value failed JSON Schema validation',
path: [] } ] }

c)Many APIs
Eg: ReplicationPolicies_Get
Polymorphic objects are not allowed, only discriminator field is getting accepted by validator.

Error in validator tool
error: Found errors in validating the response with statusCode "200" for x-ms-example "Gets the requested policy." in operation "ReplicationPolicies_Get".:

{ code: 'RESPONSE_VALIDATION_ERROR',
message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Gets the requested policy." in operation "ReplicationPolicies_Get".',
innerErrors:
[ { code: 'INVALID_RESPONSE_BODY',
errors:
[ { code: 'OBJECT_ADDITIONAL_PROPERTIES',
params:
[ [ 'activeStorageAccountId',
'replicationInterval',
'applicationConsistentSnapshotFrequencyInHours',
'recoveryPointHistoryDurationInHours' ] ],
message: 'Additional properties not allowed: activeStorageAccountId,replicationInterval,applicationConsistentSnapshotFrequencyInHours,recoveryPointHistoryDurationInHours',
path: [ 'properties', 'providerSpecificDetails' ],
description: 'Hyper V Replica Azure provider specific settings.\r\n Replicated item properties specific to the HyperVReplicaAzure replication provider.' } ],
message: 'Invalid body: Additional properties not allowed: activeStorageAccountId,replicationInterval,applicationConsistentSnapshotFrequencyInHours,recoveryPointHistoryDurationInHours',
path: [] } ] }

I have sent a mail to @veronicagg , @salameer with details on closures.

avneeshrai · 2017-05-09T19:25:47Z

@dsgouda These projects are managed by separate teams just that the provider is same. For example you can see
https://github.com/Azure/azure-rest-api-specs/tree/master/arm-recoveryservices
and
https://github.com/Azure/azure-rest-api-specs/tree/master/arm-recoveryservicesbackup

are in different directories.

avneeshrai · 2017-05-09T19:36:02Z

@dsgouda have added you to the mail thread.

veronicagg · 2017-05-09T21:59:26Z

@avneeshrai

Regarding OperationsAPIImplementation, adding Gaurav. @ravbhatnagar should this spec contain a duplicate of the operations API (the same provider has an operation here https://github.com/Azure/azure-rest-api-specs/blob/08f0f317ccb89b86f552232bd8fce02f98889463/arm-recoveryservices/2016-06-01/swagger/vaultusages.json) ?
Examples should be included even though there are failures reported for some of them (unless it completely blocks the use of the tool for the rest - like a critical exceptions). For failures that are known or new tool bugs, we would ignore the failure for now and should go away once the bug gets fixed.
a) Regarding "Maximum call stack size exceeded" issue, could you please open an issue for it at https://github.com/Azure/oav/issues (I don't think it's a known failure)
b) You should leave the examples in, we'll ignore the failures for now
c) are you seeing the polymorphic problem in the latest version of the tool? We fixed polymorphic types quite a bit, so I'd be curious to see the specific problem if you're using the latest. If so, please log an issue at https://github.com/Azure/oav/issues for us to take a look.
The examples are not only used for validation but also for documentation purposes.

veronicagg · 2017-05-09T22:23:55Z

Is this version of the service already deployed to ARM?

Regarding linter validation errors, our tool has been updated since the time of our last conversation, please review the following:

Consider flattening: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L2491
(and other locations for warning AvoidNestedProperties)
The following are currently detected as RPC violations - @ravbhatnagar for review as well.
Error at https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1975
Error https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1963
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1952
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1939
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1927
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1915
... please look at the other camel casing violations as well.
PutRequestResponseValidation https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1340
PutRequestResponseValidation https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1328
... and others on this category.

veronicagg · 2017-05-09T21:50:19Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/ResourceName"
+          },
+          {
+            "$ref": "#/para


should this be boolean? a property with the same name and description is boolean at https://github.com/Azure/azure-rest-api-specs/pull/1209/files#diff-f47a33852eff8e5696392b354d7a3636R11347

Same as above. We have exception from your side in email thread.

veronicagg · 2017-05-09T21:50:38Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/ResourceName"
+          },
+          {
+            "$ref": "#/para


should this be boolean? a property with the same name and description is boolean at https://github.com/Azure/azure-rest-api-specs/pull/1209/files#diff-f47a33852eff8e5696392b354d7a3636R11347

As we had discussed in our e-mail discussions, it was not possible for us to take any change at the service end. We had decided take up any such change whenever we come up with new version in the service. We had exception from your side on this.

veronicagg · 2017-05-09T21:53:09Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/ResourceName"
+          },
+          {
+            "$ref": "#/para


https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L2611 do you want to consider flattening here?

Same. We had decided not to go for flattening.

veronicagg · 2017-05-09T21:53:58Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/ResourceName"
+          },
+          {
+            "$ref": "#/para


https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L2599 do you want to consider flattening here?

Same. We had decided not to go for flattening.

veronicagg · 2017-05-09T21:55:23Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/ResourceName"
+          },
+          {
+            "$ref": "#/para


https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L2587 do you want to consider flattening here?

We had decided not to go for flattening and closure is in our mail thread.

veronicagg · 2017-05-09T22:19:48Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/ResourceName"
+          },
+          {
+            "$ref": "#/para


do you need this model to be different in the body compared to the response model - see https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1340

We already closed on this in our mail thread with exception from your side. For context, we had approval from ARM board which was shared on the email.
Our mail thread can be referred for details.

avneeshrai · 2017-05-10T15:09:33Z

@veronicagg

About examples:

We have added the support of examples in 7 missing APIs as you suggested (which are failing in validator with Max Call Stack Size Exceeded message as explained before)
We are still hitting all the three issues mentioned above in examples even after updating validator tool (oav):
a) Max Call Stack Size Exceeded for responses with recursive references.
b) Null values for string fields are not supported. Fails with message message: 'Expected type string but found type null'
c) Polymorphic fields are not supported and validator fails with message: "Additional properties not allowed:"

I would raise the issues as you had suggested for these scenarios.

avneeshrai · 2017-05-10T16:17:06Z

@veronicagg Please find inline responses in bolditalics for your queries.

Is this version of the service already deployed to ARM?
[AR] Yes, service version 2016-08-10 is already deployed to ARM.

Regarding linter validation errors, our tool has been updated since the time of our last conversation, please review the following:
[AR] We already have closure with exception from your end on all of the following issues. Please refer the email thread for details.

Consider flattening: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L2491
(and other locations for warning AvoidNestedProperties)
The following are currently detected as RPC violations - @ravbhatnagar for review as well.
[AR] Why is this reported as RPCVoilation? It looks to be related to nested properties that we had decided to keep as it is.

avneeshrai · 2017-05-10T16:54:58Z

Raised issues for problems with examples stated above:
Azure/oav#115
Azure/oav#116
Azure/oav#117

veronicagg · 2017-05-10T18:56:56Z

@avneeshrai Just to clarify, 1) the tools have been updated, so issues that may have been false positives before may not be now, so it's good to re-evaluate. 2) it's good to have this documented in this PR.
The flattening warning is not an RPC violations (I was referring to other ones above), and if you decided not to flatten those intentionally, it's ok. Because there are RPC violations reported, I've pulled in @ravbhatnagar in the PR review.

veronicagg · 2017-05-10T18:59:34Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+        }
+      }
+    },
+    "/Subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{resourceName}/replicationFabrics/{fabricName}/replicationvCenters/{vCenterName}/operationresults/{jobName}": {


is there a reason for this API to be exposed? How are you expecting customers to use it?

I was not really sure on inclusion of these APIs (operationresults) in spec. Only rationale i had behind including them in swagger spec was that customers who are looking at swagger spec as documentation of our APIs should get to know what all exists in our entire API surface and what API he can call if he wants to track some operation's result.
However, i don't see much utility for consumers of generated sdk. What is the recommendation here?

unless there's a specific reason to include them, we're generally not including them. For long running operations, the sdk generated code will provide methods that can poll and obtain the results as appropriate.

Have removed all operationresult apis in the latest commit.

veronicagg · 2017-05-10T19:03:14Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+        }
+      }
+    },
+    "/Subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.RecoveryServices/vaults/{resourceName}/replicationFabrics/{fabricName}/replicationStorageClassifications/{storageClassificationName}/replicationStorageClassificationMappings/{storageClassificationMappingName}/operationresults/{jobName}": {


is there a reason for this API to be exposed? How are you expecting customers to use it?
similar question for other "operationresults" apis

Same as before, I was not really sure on inclusion of these APIs (operationresults) in spec. Only rationale i had behind including them in swagger spec was that customers who are looking at swagger spec as documentation of our APIs should get to know what all exists in our entire API surface and what API he can call if he wants to track some operation's result.
However, i don't see much utility for consumers of generated sdk. What is the recommendation here?

see above, I would remove them, if you don't have a specific reason to include them.

avneeshrai · 2017-05-10T19:39:55Z

@veronicagg I get the reasons now. Thanks for clarifying :)

veronicagg · 2017-05-10T20:30:37Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            "$ref": "#/parameters/SubscriptionId"
+          },
+          {
+            "name": "fabricName",


you could define this parameter in the global parameters section and have a reference here, if you're using it multiple times. And similarly for any other parameters that you're planning to reuse.

Given the kind of usage we have for parameters like fabricName, containerName they change in general for every second operation our customers can call, to minimize confusion we have kept all parameter which would remain constant for a instance of initialized client in global list and others as local. This is also consistent with the current usage.

Ok, up to you, my suggestion was mostly for your convenience, so you didn't need to repeat and modify everywhere you needed an update in the swagger.

@veronicagg We are generating the spec automatically. So that is not an issue for us :)

veronicagg · 2017-05-10T20:33:18Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

+            }
+          }
+        },
+        "deprecated": false,


no need to include deprecated: false, as it's the default value.

Sure, removing this from all operations.

sounds good.

…n ids and 2) Taking PR comments: removing redundant deprecated field from operations.

avneeshrai · 2017-05-11T18:19:55Z

@veronicagg Addressed the comments with new commits.

ravbhatnagar · 2017-05-11T22:37:24Z

@avneeshrai - Please fix the following issues too -

https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1977
All the extra properties should go inside the properties bag.
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1207
Add the /operations API to the swagger.
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/230461195#L1963
Patch should not have a required property. Consider fixing.
When you plan on fixing the camel casing violations?

1) Introducing operations API with corresponding example. 2) Fixing required field from patch input (UpdateRecoveryPlanInput).

avneeshrai · 2017-05-12T15:55:00Z

@ravbhatnagar Taken all comments that we could. Please find the responses:

Moving extra properties inside the properties bag of 'Job' model, could not be taken because of the following reasons:
a) We had to introduce ['status,error,startTime,endTime'] properties at the top level of 'Job' because they were needed by Azure template tracking. While developing our APIs and using them through templates, templates looked for status field and other fields at the top level (absence let to failure).
This link explains the same: https://docs.microsoft.com/en-us/azure/azure-resource-manager/resource-manager-async-operations
b) Moreover, even if you suggest alternatives at this point for azure template tracking to work even after pushing these properties inside property bag we can take that only in our next service version (it being a breaking change).
c) Additional information, 'Job' is not an azure tracked resource.
Introduced operation API and corresponding example as suggested.
Fixed the issue with patch input as per suggestion.
We would be fixing all the camel casing violations in our next service version.

…d corresponding examples. 2) Converting some fields from boolean to string and making corresponding changes in examples.

avneeshrai · 2017-05-12T19:20:51Z

@veronicagg @ravbhatnagar I have responded to all the comments (fixing issues wherever possible).
Please let me know if there is any action item left on me.

veronicagg

couple more questions, inline, rest looks good to me.

veronicagg · 2017-05-12T19:20:50Z

...recoveryservicessiterecovery/2016-08-10/examples/ReplicationProtectedItems_TestFailover.json

        "networkType": "VmNetworkAsInput",
        "networkId": "/subscriptions/c183865e-6077-46f2-a3b1-deb0f4f4650a/resourceGroups/siterecoveryProd1/providers/Microsoft.Network/virtualNetworks/vnetavrai",
-        "skipTestFailoverCleanup": false,
+        "skipTestFailoverCleanup": "false",


If this is really a boolean value, should this property be represented as boolean on the swagger? then you can use boolean true/false.

@veronicagg Service is expecting this as string. Its an intentional change. We realized this while trying to validate a variance of testfailover.

veronicagg · 2017-05-12T19:24:17Z

arm-recoveryservicessiterecovery/2016-08-10/swagger/service.json

        },
        "skipTestFailoverCleanup": {
          "description": "A value indicating whether the test failover cleanup is to be skipped.",
-          "type": "boolean"


is this change intentional? is the value not a boolean?

Service is expecting this as string. Its an intentional change.

avneeshrai · 2017-05-12T19:59:48Z

@veronicagg Responded.

veronicagg

Thanks for the fixes! LGTM I'm approving from my side, we just need approval from @ravbhatnagar on the ARM side.

ravbhatnagar · 2017-05-12T21:49:09Z

@avneeshrai - The following comment from you is not correct -
a) We had to introduce ['status,error,startTime,endTime'] properties at the top level of 'Job' because they were needed by Azure template tracking. While developing our APIs and using them through templates, templates looked for status field and other fields at the top level (absence let to failure).
This link explains the same: https://docs.microsoft.com/en-us/azure/azure-resource-manager/resource-manager-async-operations
Firstly, POSTs cant be invoked from the template barring a few which are of the pattern list*. Secondly, if you are using the 202 pattern for async operations, the API should return a location header which is used to track the status of the operation. When invoked from template, the above location header is what will be used to poll for completion.
The above behavior is explained in the RPC. You do not need the above properties at the top level for the template to track the provisioning status.
This is a RPC violation. It does not matter if its a proxy resource. Please fix whenever you can. Till then you will be reported for this violation.

avneeshrai · 2017-05-12T22:07:56Z

@ravbhatnagar Thanks for the response and explanation Gaurav. That was a gap in our understanding then. Would try taking this up whenever we come up with new service version for breaking changes.

@veronicagg As now we don't have any open actionable item left, can we have this PR merged please :)

AutorestCI · 2017-05-13T06:43:15Z

No modification for Ruby

AutorestCI · 2017-05-13T06:43:23Z

No modification for NodeJS

AutorestCI · 2017-05-13T06:43:48Z

No modification for Python

Adding swagger spec for RecoveryServices.SiteRecovery service (2016-0…

2cff60c

…8-10).

msftclas added the cla-not-required label May 9, 2017

azuresdkci assigned dsgouda May 9, 2017

dsgouda suggested changes May 9, 2017

View reviewed changes

veronicagg assigned veronicagg and unassigned dsgouda May 9, 2017

veronicagg suggested changes May 9, 2017

View reviewed changes

Adding missing example support for 7 APIs.

46786da

avneeshrai mentioned this pull request May 10, 2017

Adding projects for RecoveryServices.SiteRecovery service. Azure/azure-sdk-for-net#3194

Merged

8 tasks

veronicagg assigned ravbhatnagar May 10, 2017

veronicagg suggested changes May 10, 2017

View reviewed changes

veronicagg reviewed May 10, 2017

View reviewed changes

avneeshraiusit added 2 commits May 11, 2017 17:55

1) Renaming GetTrackAsyncOperation to GetOperationResults in operatio…

f9e7be7

…n ids and 2) Taking PR comments: removing redundant deprecated field from operations.

Renaming examples to match renamed operation ids.

480f5ab

Taking PR comments:

66b1c50

1) Introducing operations API with corresponding example. 2) Fixing required field from patch input (UpdateRecoveryPlanInput).

1) Taking review comment: Removing all operationresults operations an…

bf46d38

…d corresponding examples. 2) Converting some fields from boolean to string and making corresponding changes in examples.

veronicagg suggested changes May 12, 2017

View reviewed changes

veronicagg approved these changes May 12, 2017

View reviewed changes

dsgouda approved these changes May 12, 2017

View reviewed changes

veronicagg merged commit e382d03 into Azure:master May 13, 2017

mukum mentioned this pull request Aug 17, 2017

Adding a new api to fetch the health details of the resource "vault". Apart form this this PR also contains some fixes based on some new rules added in the auto rest linter tool. #1506

Merged

10 tasks

Adding swagger spec for RecoveryServices.SiteRecovery service. #1209

Adding swagger spec for RecoveryServices.SiteRecovery service. #1209

Uh oh!

Conversation

avneeshrai commented May 9, 2017 • edited by amarzavery Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

msftclas commented May 9, 2017

Uh oh!

dsgouda left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avneeshrai commented May 9, 2017

Uh oh!

avneeshrai commented May 9, 2017

Uh oh!

avneeshrai commented May 9, 2017

Uh oh!

veronicagg commented May 9, 2017

Uh oh!

veronicagg commented May 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avneeshrai May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avneeshrai May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avneeshrai May 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avneeshrai commented May 10, 2017

Uh oh!

avneeshrai commented May 10, 2017

Uh oh!

avneeshrai commented May 10, 2017

Uh oh!

veronicagg commented May 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avneeshrai commented May 10, 2017

avneeshrai commented May 9, 2017 •

edited by amarzavery

Loading

dsgouda left a comment •

edited

Loading

avneeshrai May 10, 2017 •

edited

Loading

avneeshrai May 10, 2017 •

edited

Loading

avneeshrai May 10, 2017 •

edited

Loading

avneeshrai commented May 11, 2017 •

edited

Loading