-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … #891
Conversation
@jhendrixMSFT I am seeing 110 validation errors regarding property description in the spec. We are not describing them while defining the object but while referring. Please let us know if we need to fix this. Our current change is only few bug fixes. |
Hi @pankajsn I'm following up on your question and should have an answer for soon. |
…esource definition
@pankajsn sorry for the delay. We'll need these validation errors to be fixed, the reason being that they're used for generating docs during SDK generation. |
@jhendrixMSFT validation errors are fixed. |
Hi Joel,
I have fixed the validation warnings.
Please let us know if there is any other pending action item on us. We have respective SDK and PowerShell updates pending on this, that we are targeting to release with February milestone.
Regards,
~Pankaj
From: Joel Hendrix [mailto:notifications@github.com]
Sent: Thursday, February 2, 2017 8:24 AM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@pankajsn<https://github.com/pankajsn> sorry for the delay. We'll need these validation errors to be fixed, the reason being that they're used for generating docs during SDK generation.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc0-rObVHvfV2uOG8myUOymwYHjqZks5rYgM6gaJpZM4LyBtI>.
|
"KeyType": { | ||
"type": "string", | ||
"enum": [ | ||
"NotSpecified", |
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.
Breaking change on several counts.
- New value is added to the list
- New value is added at the top of the list, instead of it being in the bottom. Previous version had Primary and Secondary.
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 like this hasn't been addressed yet?
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 like this value was already there. @amarzavery did you mean a different enum?
@pankajsn - We have hooked up a bunch of validation tools as a part of our CI . The linter shows some warnings. Please make sure you address them before the PR can be merged.
|
Hi @pankajsn, Can you please provide examples for request and response using the extension x-ms-examples? This will
You can find more info about the extension and an example swagger spec over here. |
"description": "No Content" | ||
} | ||
} | ||
} | ||
} | ||
}, | ||
"definitions": { | ||
"Resource": { |
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.
@pankajsn There is an issue #898 reported by a customer which is correct. Can you please resolve this issue, by accurately representing the properties of "Resource" "readOnly": true
.
- "id", "name" and "type" should be marked
"readOnly": true
as per ARM guidelines. - Shouldn't "location" be a required property
"required": ["location"]
?
@Amar Zavery<mailto:amzavery@microsoft.com>
These are little confusing or not accurate. Please find CIL and help us understanding these warning.
From: Amar Zavery [mailto:notifications@github.com]
Sent: Monday, February 6, 2017 2:32 PM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@pankajsn<https://github.com/pankajsn> - We have hooked up a bunch of validation tools as a part of our CI .
The linter shows some warnings. Please make sure you address them before the PR can be merged.
Warnings can be found here<https://travis-ci.org/Azure/azure-rest-api-specs/jobs/199003272#L2326>.
AutoRest Linter validation:
Executing: mono ./AutoRest.*/tools/AutoRest.exe -CodeGenerator None -I arm-logic/2016-06-01/swagger/logic.json
WARNING: NonAppJsonTypeWarning - Media types other than 'application/json' has limited support
Path: #/Consumes[2]
WARNING: NonAppJsonTypeWarning - Media types other than 'application/json' has limited support
Path: #/Produces[2]
WARNING: OperationsAPIImplementationValidation - Operations API must be implemented for the service.
Path: #/Paths
Above warnings are very generic. Are these valid for our spec?
WARNING: ResourceModelValidation - The id, name, type, location and tags properties of the Resource must be present with id, name and type as read-only
Path: #/Definitions
In progress
WARNING: TrackedResourceValidation - Tracked Resource failing validation is: Workflow. Validation Failed: 4.
A Tracked Resource must have:
1. A Get Operation
Workflows has get operation see Line no#134
2. A ListByResourceGroup operation with x-ms-pageable extension and
Operation Workflows_ListByResourceGroup has x-ms-pageable extension. see line #128
3. A ListBySubscriptionId operation with x-ms-pageable extension.
Operation Workflows_ListBySubscription has x-ms-pageable extension. see line #77
4. Type, Location, Tags should not be used in the properties.
Path: #/Definitions
These properties are not defined in the Workflow but inheriting from Resource. Is it not correct ? See line #2548
WARNING: SkuModelValidation - Sku Model is not valid. A Sku model must have name property. It can also have tier, size, family, capacity as optional properties.
Path: #/Definitions
Sku has name property. See line #3276
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc4WxIA-jCSmlctJGSzCMSndb8yMnks5rZ592gaJpZM4LyBtI>.
|
I sat down with @amarzavery regarding your questions, below are the answers. Let me know if you have further questions. NonAppJsonTypeWarning - can you please verify that your RP actually consumes and produces text/json in addition to application/json? OperationsAPIImplementationValidation - you need to implement the operations API. For an example please see https://github.com/Azure/azure-rest-api-specs/blob/master/arm-cdn/2016-10-02/swagger/cdn.json#L1462 TrackedResourceValidation - the output here is unclear and we will work to fix that. Only item 4 (type, location etc) has an issue (that's what the "Validation Failed: 4" means) and this appears to be a bug in the validator. You can ignore this for now. SkuModelValidation - another unclear error message which we'll address. The "name" field needs to be marked as required. |
@joel Hendrix<mailto:jhendrix@microsoft.com> @Amar Zavery<mailto:amzavery@microsoft.com>
I have fixed below warnings (except NonAppJsonTypeWarning: I am following up on that). I hope this is not blocking this PR as this change is not introduced as part of this PR. Once confirmed, I will start a new PR for that.
From: Joel Hendrix [mailto:notifications@github.com]
Sent: Tuesday, February 7, 2017 1:34 PM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
I sat down with @amarzavery<https://github.com/amarzavery> regarding your questions, below are the answers. Let me know if you have further questions.
NonAppJsonTypeWarning - can you please verify that your RP actually consumes and produces text/json in addition to application/json?
OperationsAPIImplementationValidation - you need to implement the operations API. For an example please see https://github.com/Azure/azure-rest-api-specs/blob/master/arm-cdn/2016-10-02/swagger/cdn.json#L1462
TrackedResourceValidation - the output here is unclear and we will work to fix that. Only item #4<#4> (type, location etc) has an issue (that's what the "Validation Failed: 4" means) and this appears to be a bug in the validator. You can ignore this for now.
SkuModelValidation - another unclear error message which we'll address. The "name" field needs to be marked as required.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc-CkGWZeNFier2GvHEhU8M_q_Bznks5raONkgaJpZM4LyBtI>.
|
RP does not support text/json. Updated the spec. Please review the PR.
From: Pankaj Singh Negi
Sent: Tuesday, February 7, 2017 4:47 PM
To: 'Azure/azure-rest-api-specs' <reply@reply.github.com>; Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>; Joel Hendrix <jhendrix@microsoft.com>; Amar Zavery <amzavery@microsoft.com>
Cc: Mention <mention@noreply.github.com>; Vinay Singh <vinaysin@microsoft.com>
Subject: RE: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@joel Hendrix<mailto:jhendrix@microsoft.com> @Amar Zavery<mailto:amzavery@microsoft.com>
I have fixed below warnings (except NonAppJsonTypeWarning: I am following up on that). I hope this is not blocking this PR as this change is not introduced as part of this PR. Once confirmed, I will start a new PR for that.
From: Joel Hendrix [mailto:notifications@github.com]
Sent: Tuesday, February 7, 2017 1:34 PM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
I sat down with @amarzavery<https://github.com/amarzavery> regarding your questions, below are the answers. Let me know if you have further questions.
NonAppJsonTypeWarning - can you please verify that your RP actually consumes and produces text/json in addition to application/json?
OperationsAPIImplementationValidation - you need to implement the operations API. For an example please see https://github.com/Azure/azure-rest-api-specs/blob/master/arm-cdn/2016-10-02/swagger/cdn.json#L1462
TrackedResourceValidation - the output here is unclear and we will work to fix that. Only item #4<#4> (type, location etc) has an issue (that's what the "Validation Failed: 4" means) and this appears to be a bug in the validator. You can ignore this for now.
SkuModelValidation - another unclear error message which we'll address. The "name" field needs to be marked as required.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc-CkGWZeNFier2GvHEhU8M_q_Bznks5raONkgaJpZM4LyBtI>.
|
"tags": [ | ||
"IntegrationAccountSchemas" | ||
], | ||
"operationId": "IntegrationAccountSchemas_List", |
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.
The current version of 2016-06-01 api-version does not have these APIs in the public repo. Hence they are new. I would like them to follow the naming convention right from the start, so that there are no breaking changes w.r.t naming.
For nested resources the convention is: _ListByImmediateParent.
So this should be:
Schemas_ListByIntegrationAccounts
. This will make things consistent with other APIs in Azure. The customer need not learn new things when he moves from one RP to another in Azure.
For reference: Here is the convention for naming
M1004: List operations MUST use consistent names and semantics. List operations MUST NOT use any other names. Consistent names and semantics are:
- List() - lists all resources under a subscription.
- ListByResourceGroup() - list all resources in a resource group within a subscription.
- ListByParent() - where "Parent" is a context specific suffix. It lists all resource under a parent.
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.
fixed
"tags": [ | ||
"IntegrationAccountSchemas" | ||
], | ||
"operationId": "IntegrationAccountSchemas_Get", |
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.
General comment: The method Group (part before the underscore) should match with the nested resource type in the url.
Schemas_Get
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.
fixed
"tags": [ | ||
"IntegrationAccountMaps" | ||
], | ||
"operationId": "IntegrationAccountMaps_List", |
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.
Maps_ListByIntegrationAccounts
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.
fixed
"tags": [ | ||
"IntegrationAccountMaps" | ||
], | ||
"operationId": "IntegrationAccountMaps_Get", |
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.
Same as above: General comment: Maps_Get
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.
fixed
"tags": [ | ||
"IntegrationAccountPartners" | ||
], | ||
"operationId": "IntegrationAccountPartners_List", |
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.
same as above
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.
fixed
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
required properties?
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
enum?
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
any required or readOnly properties over here?
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.
fixed across
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
What do you expect the customer to put over here? Is "FooBar" valid? Can the descriptions be more meaningful with some example values.
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
General comment please provide required and readOnly properties of all the model definitions accurately. It is hard to believe that everything is optional and free flowing.
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.
fixed accross
@amarzavery
These are already existing operations, looks like RP (/providers/Microsoft.Logic/operations) is not updated with the full list of operations.
Our primary object of the current PR is to unblock few customers by fixing couple of PowerShell bugs. Including all these changes in the existing code, will take some extra time and we don’t want to miss the current release cycle of PowerShell (code complete by 13th feb). Please suggest how can we accommodate this.
From: Amar Zavery [mailto:notifications@github.com]
Sent: Wednesday, February 8, 2017 10:23 AM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@amarzavery requested changes on this pull request.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/CallbackUrl"
+ }
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/schemas": {
+ "get": {
+ "tags": [
+ "IntegrationAccountSchemas"
+ ],
+ "operationId": "IntegrationAccountSchemas_List",
The current version of 2016-06-01 api-version does not have these APIs in the public repo. Hence they are new. I would like them to follow the naming convention right from the start, so that there are no breaking changes w.r.t naming.
For nested resources the convention is: _ListByImmediateParent.
So this should be:
Schemas_ListByIntegrationAccounts. This will make things consistent with other APIs in Azure. The customer need not learn new things when he moves from one RP to another in Azure.
For reference: Here is the convention for naming<https://github.com/Azure/azure-rest-api-specs/blob/master/documentation/swagger-checklist.md#naming---swagger-checklist>
M1004: List operations MUST use consistent names and semantics. List operations MUST NOT use any other names. Consistent names and semantics are:
- List() - lists all resources under a subscription.
- ListByResourceGroup() - list all resources in a resource group within a subscription.
- ListByParent() - where "Parent" is a context specific suffix. It lists all resource under a parent.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "$ref": "#/definitions/IntegrationAccountSchemaListResult"
+ }
+ }
+ },
+ "x-ms-pageable": {
+ "nextLinkName": "nextLink"
+ },
+ "x-ms-odata": "#/definitions/IntegrationAccountSchemaFilter"
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/schemas/{schemaName}": {
+ "get": {
+ "tags": [
+ "IntegrationAccountSchemas"
+ ],
+ "operationId": "IntegrationAccountSchemas_Get",
General comment: The method Group (part before the underscore) should match with the nested resource type in the url.
Schemas_Get
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK"
+ },
+ "204": {
+ "description": "No Content"
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/maps": {
+ "get": {
+ "tags": [
+ "IntegrationAccountMaps"
+ ],
+ "operationId": "IntegrationAccountMaps_List",
Maps_ListByIntegrationAccounts
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "$ref": "#/definitions/IntegrationAccountMapListResult"
+ }
+ }
+ },
+ "x-ms-pageable": {
+ "nextLinkName": "nextLink"
+ },
+ "x-ms-odata": "#/definitions/IntegrationAccountMapFilter"
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/maps/{mapName}": {
+ "get": {
+ "tags": [
+ "IntegrationAccountMaps"
+ ],
+ "operationId": "IntegrationAccountMaps_Get",
Same as above: General comment: Maps_Get
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK"
+ },
+ "204": {
+ "description": "No Content"
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/partners": {
+ "get": {
+ "tags": [
+ "IntegrationAccountPartners"
+ ],
+ "operationId": "IntegrationAccountPartners_List",
same as above
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ }
+ ],
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/IntegrationAccountPartner"
+ }
+ }
+ }
+ },
+ "put": {
+ "tags": [
+ "IntegrationAccountPartners"
+ ],
+ "operationId": "IntegrationAccountPartners_CreateOrUpdate",
Partners_CreateOrUpdate
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "id": {
+ "type": "string",
+ "readOnly": true,
+ "description": "The resource id."
+ },
+ "name": {
+ "type": "string",
+ "readOnly": true,
+ "description": "Gets the resource name."
+ },
+ "type": {
+ "type": "string",
+ "readOnly": true,
+ "description": "Gets the resource type."
+ },
+ "location": {
What about location being required?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "id": {
+ "type": "string",
+ "description": "The resource id."
+ }
+ },
+ "x-ms-azure-resource": true,
+ "description": "The sub resource type."
+ },
+ "Object": {
+ "type": "object",
+ "properties": {}
+ },
+ "ResourceReference": {
+ "type": "object",
+ "properties": {
+ "id": {
Shouldn't the id be readOnly as well?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Why do you need this model? How is this different from Resource? Why can't IntegrationAccount be an allOf on the Resource?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Is metadata a free form object or a dictionary <string, someObject>
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What is the unit?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
should this be an enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
This looks weird. The IntegrationAccountProperties does not have "properties":{} object. Is the property bag empty?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What are the required properties over here? If the customer passes an empty object, what will the service do, send a 400 or assume default values for some of the properties? Which means that those properties would either be required or will have a default value in swagger spec.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Why is "NotAfter" in PascalCase and "keyType" in camelCase? Are you sure that is the correct casing on wire (verify via Fiddler)? Don't look at the C# code on server side that does not reflect what is on the wire.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ },
+ {
+ "name": "parameters",
+ "description": "The callback URL parameters.",
+ "in": "body",
+ "required": true,
+ "schema": {
+ "$ref": "#/definitions/ListCallbackUrlParameters"
+ }
+ }
+ ],
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/CallbackUrl"
The name of the operation is List where as the return type of the object is a string. The customer would expect that the service would return an array or an object that has some property that is of type array. Please fix the name of the method to _GetCallbackUrl
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Is this a string or an array of strings?
See if you would have given x-ms-examples, our job would have been easy and the CI would have caught if something is wrong.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
allOf on the Resource should be fine.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What is the schema of content. How useful are free form objects to customers?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Are there any required properties over here? What do you expect the customer to send bare minimum while creating (PUT) on the "IntegrationAccountSchema"
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
should this be an enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
missing description
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
enum? How would the customer know what to put over here while creating the Map? How important is the usability of the API? REST API documentation will be generated from swagger. The description of the properties are not that helpful. Where do you expect the customer to go to find out what can he/she provide over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
missing description
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
what are the required properties over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
required properties?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
any required or readOnly properties over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What do you expect the customer to put over here? Is "FooBar" valid? Can the descriptions be more meaningful with some example values.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
General comment please provide required and readOnly properties of all the model definitions accurately. It is hard to believe that everything is optional and free flowing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc5ZbItdUVLl-tnhhWUVZX03PEXFgks5raggDgaJpZM4LyBtI>.
|
@pankajsn If there's a tight deadline then perhaps it makes sense to update this PR (or close this one and create a new one, whichever is easier) with only the necessary bug fixes for PowerShell (IIRC it was just a few lines of changes), and do this larger refactoring/API additions in a separate PR. Thoughts? |
@joel Hendrix<mailto:jhendrix@microsoft.com>
Other than bug fixes, we want to merge the older version of spec into the latest one. This is just to ensure that customer hits the latest api version. Other than that we fixed couple of linter warnings. Why don’t we pull this change-set as this is already reviewed. Please suggest.
For all the other refactoring related fixes, we can pull into a separate PR. Undoing will also take some extra effort at this point.
From: Joel Hendrix [mailto:notifications@github.com]
Sent: Wednesday, February 8, 2017 11:25 AM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@pankajsn<https://github.com/pankajsn> If there's a tight deadline then perhaps it makes sense to update this PR (or close this one and create a new one, whichever is easier) with only the necessary bug fixes for PowerShell (IIRC it was just a few lines of changes), and do this larger refactoring/API additions in a separate PR. Thoughts?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc6xeQzUUoHZikNwZP5FABQNp6yMYks5rahZvgaJpZM4LyBtI>.
|
@joel Hendrix<mailto:jhendrix@microsoft.com>
As discussed, we will fix all the comments as part of current PR.
@Amar Zavery<mailto:amzavery@microsoft.com>
Some of the comments below are not pointing to the correct line numbers hence not clear. Could you please point it correctly ?
From: Pankaj Singh Negi
Sent: Wednesday, February 8, 2017 11:12 AM
To: 'Azure/azure-rest-api-specs' <reply@reply.github.com>; Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Mention <mention@noreply.github.com>; Vinay Singh <vinaysin@microsoft.com>
Subject: RE: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@amarzavery
These are already existing operations, looks like RP (/providers/Microsoft.Logic/operations) is not updated with the full list of operations.
Our primary object of the current PR is to unblock few customers by fixing couple of PowerShell bugs. Including all these changes in the existing code, will take some extra time and we don’t want to miss the current release cycle of PowerShell (code complete by 13th feb). Please suggest how can we accommodate this.
From: Amar Zavery [mailto:notifications@github.com]
Sent: Wednesday, February 8, 2017 10:23 AM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@amarzavery requested changes on this pull request.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/CallbackUrl"
+ }
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/schemas": {
+ "get": {
+ "tags": [
+ "IntegrationAccountSchemas"
+ ],
+ "operationId": "IntegrationAccountSchemas_List",
The current version of 2016-06-01 api-version does not have these APIs in the public repo. Hence they are new. I would like them to follow the naming convention right from the start, so that there are no breaking changes w.r.t naming.
For nested resources the convention is: _ListByImmediateParent.
So this should be:
Schemas_ListByIntegrationAccounts. This will make things consistent with other APIs in Azure. The customer need not learn new things when he moves from one RP to another in Azure.
For reference: Here is the convention for naming<https://github.com/Azure/azure-rest-api-specs/blob/master/documentation/swagger-checklist.md#naming---swagger-checklist>
M1004: List operations MUST use consistent names and semantics. List operations MUST NOT use any other names. Consistent names and semantics are:
- List() - lists all resources under a subscription.
- ListByResourceGroup() - list all resources in a resource group within a subscription.
- ListByParent() - where "Parent" is a context specific suffix. It lists all resource under a parent.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "$ref": "#/definitions/IntegrationAccountSchemaListResult"
+ }
+ }
+ },
+ "x-ms-pageable": {
+ "nextLinkName": "nextLink"
+ },
+ "x-ms-odata": "#/definitions/IntegrationAccountSchemaFilter"
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/schemas/{schemaName}": {
+ "get": {
+ "tags": [
+ "IntegrationAccountSchemas"
+ ],
+ "operationId": "IntegrationAccountSchemas_Get",
General comment: The method Group (part before the underscore) should match with the nested resource type in the url.
Schemas_Get
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK"
+ },
+ "204": {
+ "description": "No Content"
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/maps": {
+ "get": {
+ "tags": [
+ "IntegrationAccountMaps"
+ ],
+ "operationId": "IntegrationAccountMaps_List",
Maps_ListByIntegrationAccounts
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "$ref": "#/definitions/IntegrationAccountMapListResult"
+ }
+ }
+ },
+ "x-ms-pageable": {
+ "nextLinkName": "nextLink"
+ },
+ "x-ms-odata": "#/definitions/IntegrationAccountMapFilter"
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/maps/{mapName}": {
+ "get": {
+ "tags": [
+ "IntegrationAccountMaps"
+ ],
+ "operationId": "IntegrationAccountMaps_Get",
Same as above: General comment: Maps_Get
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK"
+ },
+ "204": {
+ "description": "No Content"
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/partners": {
+ "get": {
+ "tags": [
+ "IntegrationAccountPartners"
+ ],
+ "operationId": "IntegrationAccountPartners_List",
same as above
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ }
+ ],
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/IntegrationAccountPartner"
+ }
+ }
+ }
+ },
+ "put": {
+ "tags": [
+ "IntegrationAccountPartners"
+ ],
+ "operationId": "IntegrationAccountPartners_CreateOrUpdate",
Partners_CreateOrUpdate
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "id": {
+ "type": "string",
+ "readOnly": true,
+ "description": "The resource id."
+ },
+ "name": {
+ "type": "string",
+ "readOnly": true,
+ "description": "Gets the resource name."
+ },
+ "type": {
+ "type": "string",
+ "readOnly": true,
+ "description": "Gets the resource type."
+ },
+ "location": {
What about location being required?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "id": {
+ "type": "string",
+ "description": "The resource id."
+ }
+ },
+ "x-ms-azure-resource": true,
+ "description": "The sub resource type."
+ },
+ "Object": {
+ "type": "object",
+ "properties": {}
+ },
+ "ResourceReference": {
+ "type": "object",
+ "properties": {
+ "id": {
Shouldn't the id be readOnly as well?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Why do you need this model? How is this different from Resource? Why can't IntegrationAccount be an allOf on the Resource?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Is metadata a free form object or a dictionary <string, someObject>
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What is the unit?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
should this be an enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
This looks weird. The IntegrationAccountProperties does not have "properties":{} object. Is the property bag empty?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What are the required properties over here? If the customer passes an empty object, what will the service do, send a 400 or assume default values for some of the properties? Which means that those properties would either be required or will have a default value in swagger spec.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Why is "NotAfter" in PascalCase and "keyType" in camelCase? Are you sure that is the correct casing on wire (verify via Fiddler)? Don't look at the C# code on server side that does not reflect what is on the wire.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ },
+ {
+ "name": "parameters",
+ "description": "The callback URL parameters.",
+ "in": "body",
+ "required": true,
+ "schema": {
+ "$ref": "#/definitions/ListCallbackUrlParameters"
+ }
+ }
+ ],
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/CallbackUrl"
The name of the operation is List where as the return type of the object is a string. The customer would expect that the service would return an array or an object that has some property that is of type array. Please fix the name of the method to _GetCallbackUrl
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Is this a string or an array of strings?
See if you would have given x-ms-examples, our job would have been easy and the CI would have caught if something is wrong.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
allOf on the Resource should be fine.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What is the schema of content. How useful are free form objects to customers?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Are there any required properties over here? What do you expect the customer to send bare minimum while creating (PUT) on the "IntegrationAccountSchema"
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
should this be an enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
missing description
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
enum? How would the customer know what to put over here while creating the Map? How important is the usability of the API? REST API documentation will be generated from swagger. The description of the properties are not that helpful. Where do you expect the customer to go to find out what can he/she provide over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
missing description
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
what are the required properties over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
required properties?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
any required or readOnly properties over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What do you expect the customer to put over here? Is "FooBar" valid? Can the descriptions be more meaningful with some example values.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
General comment please provide required and readOnly properties of all the model definitions accurately. It is hard to believe that everything is optional and free flowing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc5ZbItdUVLl-tnhhWUVZX03PEXFgks5raggDgaJpZM4LyBtI>.
|
@Amar Zavery<mailto:amzavery@microsoft.com> @joel Hendrix<mailto:jhendrix@microsoft.com>
Below highlighted comments are not pointing the correct location hence not very clear. Is there any way to update these ?
From: Amar Zavery [mailto:notifications@github.com]
Sent: Wednesday, February 8, 2017 10:23 AM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@amarzavery requested changes on this pull request.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/CallbackUrl"
+ }
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/schemas": {
+ "get": {
+ "tags": [
+ "IntegrationAccountSchemas"
+ ],
+ "operationId": "IntegrationAccountSchemas_List",
The current version of 2016-06-01 api-version does not have these APIs in the public repo. Hence they are new. I would like them to follow the naming convention right from the start, so that there are no breaking changes w.r.t naming.
For nested resources the convention is: _ListByImmediateParent.
So this should be:
Schemas_ListByIntegrationAccounts. This will make things consistent with other APIs in Azure. The customer need not learn new things when he moves from one RP to another in Azure.
For reference: Here is the convention for naming<https://github.com/Azure/azure-rest-api-specs/blob/master/documentation/swagger-checklist.md#naming---swagger-checklist>
M1004: List operations MUST use consistent names and semantics. List operations MUST NOT use any other names. Consistent names and semantics are:
- List() - lists all resources under a subscription.
- ListByResourceGroup() - list all resources in a resource group within a subscription.
- ListByParent() - where "Parent" is a context specific suffix. It lists all resource under a parent.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "$ref": "#/definitions/IntegrationAccountSchemaListResult"
+ }
+ }
+ },
+ "x-ms-pageable": {
+ "nextLinkName": "nextLink"
+ },
+ "x-ms-odata": "#/definitions/IntegrationAccountSchemaFilter"
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/schemas/{schemaName}": {
+ "get": {
+ "tags": [
+ "IntegrationAccountSchemas"
+ ],
+ "operationId": "IntegrationAccountSchemas_Get",
General comment: The method Group (part before the underscore) should match with the nested resource type in the url.
Schemas_Get
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK"
+ },
+ "204": {
+ "description": "No Content"
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/maps": {
+ "get": {
+ "tags": [
+ "IntegrationAccountMaps"
+ ],
+ "operationId": "IntegrationAccountMaps_List",
Maps_ListByIntegrationAccounts
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "$ref": "#/definitions/IntegrationAccountMapListResult"
+ }
+ }
+ },
+ "x-ms-pageable": {
+ "nextLinkName": "nextLink"
+ },
+ "x-ms-odata": "#/definitions/IntegrationAccountMapFilter"
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/maps/{mapName}": {
+ "get": {
+ "tags": [
+ "IntegrationAccountMaps"
+ ],
+ "operationId": "IntegrationAccountMaps_Get",
Same as above: General comment: Maps_Get
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "responses": {
+ "200": {
+ "description": "OK"
+ },
+ "204": {
+ "description": "No Content"
+ }
+ }
+ }
+ },
+ "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Logic/integrationAccounts/{integrationAccountName}/partners": {
+ "get": {
+ "tags": [
+ "IntegrationAccountPartners"
+ ],
+ "operationId": "IntegrationAccountPartners_List",
same as above
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ }
+ ],
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/IntegrationAccountPartner"
+ }
+ }
+ }
+ },
+ "put": {
+ "tags": [
+ "IntegrationAccountPartners"
+ ],
+ "operationId": "IntegrationAccountPartners_CreateOrUpdate",
Partners_CreateOrUpdate
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "id": {
+ "type": "string",
+ "readOnly": true,
+ "description": "The resource id."
+ },
+ "name": {
+ "type": "string",
+ "readOnly": true,
+ "description": "Gets the resource name."
+ },
+ "type": {
+ "type": "string",
+ "readOnly": true,
+ "description": "Gets the resource type."
+ },
+ "location": {
What about location being required?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "id": {
+ "type": "string",
+ "description": "The resource id."
+ }
+ },
+ "x-ms-azure-resource": true,
+ "description": "The sub resource type."
+ },
+ "Object": {
+ "type": "object",
+ "properties": {}
+ },
+ "ResourceReference": {
+ "type": "object",
+ "properties": {
+ "id": {
Shouldn't the id be readOnly as well?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Why do you need this model? How is this different from Resource? Why can't IntegrationAccount be an allOf on the Resource?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Is metadata a free form object or a dictionary <string, someObject>
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What is the unit?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
should this be an enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
This looks weird. The IntegrationAccountProperties does not have "properties":{} object. Is the property bag empty?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What are the required properties over here? If the customer passes an empty object, what will the service do, send a 400 or assume default values for some of the properties? Which means that those properties would either be required or will have a default value in swagger spec.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Why is "NotAfter" in PascalCase and "keyType" in camelCase? Are you sure that is the correct casing on wire (verify via Fiddler)? Don't look at the C# code on server side that does not reflect what is on the wire.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ },
+ {
+ "name": "parameters",
+ "description": "The callback URL parameters.",
+ "in": "body",
+ "required": true,
+ "schema": {
+ "$ref": "#/definitions/ListCallbackUrlParameters"
+ }
+ }
+ ],
+ "responses": {
+ "200": {
+ "description": "OK",
+ "schema": {
+ "$ref": "#/definitions/CallbackUrl"
The name of the operation is List where as the return type of the object is a string. The customer would expect that the service would return an array or an object that has some property that is of type array. Please fix the name of the method to _GetCallbackUrl
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Is this a string or an array of strings?
See if you would have given x-ms-examples, our job would have been easy and the CI would have caught if something is wrong.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
allOf on the Resource should be fine.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What is the schema of content. How useful are free form objects to customers?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
Are there any required properties over here? What do you expect the customer to send bare minimum while creating (PUT) on the "IntegrationAccountSchema"
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
should this be an enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
missing description
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
enum? How would the customer know what to put over here while creating the Map? How important is the usability of the API? REST API documentation will be generated from swagger. The description of the properties are not that helpful. Where do you expect the customer to go to find out what can he/she provide over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
missing description
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
what are the required properties over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
required properties?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
enum?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
any required or readOnly over here?
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
What do you expect the customer to put over here? Is "FooBar" valid? Can the descriptions be more meaningful with some example values.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891 (review)>:
+ "type": "object",
+ "properties": {
+ "algorithm": {
+ "type": "string",
+ "description": "The algorithm."
+ },
+ "value": {
+ "type": "string",
+ "description": "The value."
+ }
+ },
+ "description": "The content hash."
+ },
+ "Correlation": {
+ "type": "object",
+
General comment please provide required and readOnly properties of all the model definitions accurately. It is hard to believe that everything is optional and free flowing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc5ZbItdUVLl-tnhhWUVZX03PEXFgks5raggDgaJpZM4LyBtI>.
|
@pankajsn - I am sorry, cannot do anything about it. It is a github issue. As per github guidelines, it is expected that the author resolves all the comments made by the reviewer before making a new commit. With the new commit the line numbers change and nothing can be done about it. |
…tributes and others
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
I was referring to this thing "content". What is the schema? Is it free form?
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
What is the qualifier? Is this an enum? How would the customer know what to provide?
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
Can you provide an example in the description of qualifier and value
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
Are there any required properties in "AS2OneWayAgreement"?
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
Are there any required properties in "AS2ProtocolSettings"? I mean if the user provides an empty object what would the service do? Accept it and apply defaults or send a 400?
}, | ||
"Correlation": { | ||
"type": "object", | ||
|
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.
same as above. The service is having huge model definitions. But everything is optional. It looks like garbage in garbage out. There is no way to know what should be provided on a bear minimum level
@jhendrixMSFT this is fixed in the latest iteration.
From: Joel Hendrix [mailto:notifications@github.com]
Sent: Friday, February 10, 2017 12:41 PM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@jhendrixMSFT commented on this pull request.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891>:
+ "id": {
+ "type": "string",
+ "description": "The resource id."
+ }
+ },
+ "x-ms-azure-resource": true,
+ "description": "The sub resource type."
+ },
+ "Object": {
+ "type": "object",
+ "properties": {}
+ },
+ "ResourceReference": {
+ "type": "object",
+ "properties": {
+ "id": {
I don't see this marked as read-only, has that fix not been pushed yet?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc9g1wDn-91Em63fpwrplu45R9h-oks5rbMtggaJpZM4LyBtI>.
|
@Amar Zavery<mailto:amzavery@microsoft.com> @joel Hendrix<mailto:jhendrix@microsoft.com>
Can we please complete this PR ? Also, please let me know if there are more comments.
From: Joel Hendrix [mailto:notifications@github.com]
Sent: Monday, February 13, 2017 11:31 AM
To: Azure/azure-rest-api-specs <azure-rest-api-specs@noreply.github.com>
Cc: Pankaj Singh Negi <Pankaj.Negi@microsoft.com>; Mention <mention@noreply.github.com>
Subject: Re: [Azure/azure-rest-api-specs] [Logic App Integration Account][Bug Fix] : Swagger spec updated as per the latest changes in the RP … (#891)
@jhendrixMSFT commented on this pull request.
________________________________
In arm-logic/2016-06-01/swagger/logic.json<#891>:
+ "Int",
+ "Float",
+ "Bool",
+ "Array",
+ "Object",
+ "SecureObject"
+ ],
+ "x-ms-enum": {
+ "name": "ParameterType",
+ "modelAsString": false
+ }
+ },
+ "KeyType": {
+ "type": "string",
+ "enum": [
+ "NotSpecified",
Looks like this value was already there. @amarzavery<https://github.com/amarzavery> did you mean a different enum?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#891>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APZfc-Si9lbgfNeTNT-TOrw0BZALin80ks5rcK9hgaJpZM4LyBtI>.
|
@amarzavery I believe you're good with the current iteration, if so can you please sign off on this PR so we can merge it? |
No modification for Ruby |
No modification for Python |
No modification for NodeJS |
…definitions.
Issue fixed: As the LogicApps and Integration Account are in GA, swagger spec is merged
Issue fixed: HashingAlgorithm enum in Azure SDK swagger spec is not updated with new values.
Issue fixed: Not able to create New-AzureRmIntegrationAccountAgreement with envelopeOverrides/responsibleAgencyCode due to RP definition updates.
Issue fixed: Integration Account Sku name values fixed as per review comments.
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
api-version
in the path should match theapi-version
in the spec).Quality of Swagger