Add new version for proactive detection config#3224
Add new version for proactive detection config#3224aviled wants to merge 7 commits intoAzure:masterfrom
Conversation
Automation for azure-sdk-for-rubyNothing to generate for azure-sdk-for-ruby |
Automation for azure-sdk-for-pythonNothing to generate for azure-sdk-for-python |
Automation for azure-libraries-for-javaNothing to generate for azure-libraries-for-java |
Automation for azure-sdk-for-nodeA PR has been created for you: |
Automation for azure-sdk-for-goA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
anuchandy
left a comment
anuchandy
left a comment
There was a problem hiding this comment.
@aviled there is CI failure - with error message DuplicateModelCollsion -. This detects invalid SDK situation that we weren’t detecting before: if two files that are in the same “tag” in the Readme defines the same model name Foo, both object have to be exactly (at the comma exactly level) the same. Here we have Resource model defined in webTests_API.json, components_API.json and workbooks_API.json, it seems these 3 definitions are not same. could you fix it?
|
@aviled can you fix the error reported by CI (see previous comments) ? |
Automation for azure-sdk-for-javaNothing to generate for azure-sdk-for-java |
anuchandy
left a comment
anuchandy
left a comment
There was a problem hiding this comment.
Please model patch payload correctly as described in the email
|
Looking in Eric Chong @ericc1103 owner of workbooks_api-json spec to address ARM violation. ARM-Violation: "Properties of a PATCH request body must not be required. PATCH operation: 'Workbook_Update' Model Definition: 'WorkbookResource' Property: 'location'" 'WorkbookResource' model inherits from ‘Resource’ (for which we marked location as required), this model is used for both PATCH and PUT. It is required that all the properties of model representing PATCH operation has to be optional. This also indirectly means service should not enforce any requiredness in the PATCH payload. In-order to fix this, we need to define a new model representing Workbook PATCH payload (with all properties optional). The PATCH payload model looks something like WorkbookUpdateParameters as shown below. But please note that only you know what are the PATCH-able properties of workbook so you will need to update (by adding or removing properties) the below model accordingly. "WorkbookUpdateParameters": {
"properties": {
"kind": {
"type": "string",
"description": "The kind of workbook. Choices are user and shared.",
"enum": [
"user",
"shared"
],
"x-ms-enum": {
"name": "SharedTypeKind",
"modelAsString": true
}
},
"tags": {
"type": "object",
"additionalProperties": {
"type": "string"
},
"description": "Resource tags"
},
"properties": {
"x-ms-client-flatten": true,
"description": "Metadata describing a web test for an Azure resource.",
"$ref": "#/definitions/WorkbookPropertiesUpdateParameters"
}
},
"description": "The parameters that can be provided when updating workbook properties properties."
},
"WorkbookPropertiesUpdateParameters": {
"description": "Properties that contain a workbook.",
"properties": {
"name": {
"type": "string",
"description": "The user-defined name of the workbook."
},
"serializedData": {
"type": "string",
"description": "Configuration of this particular workbook. Configuration data is a string containing valid JSON"
},
"version": {
"type": "string",
"description": "This instance's version of the data model. This can change as new features are added that can be marked workbook."
},
"workbookId": {
"type": "string",
"description": "Internally assigned unique id of the workbook definition."
},
"kind": {
"description": "Enum indicating if this workbook definition is owned by a specific user or is shared between all users with access to the Application Insights component.",
"x-ms-client-name": "SharedTypeKind",
"default": "shared",
"type": "string",
"enum": [
"shared",
"user"
],
"x-ms-enum": {
"name": "SharedTypeKind",
"modelAsString": true
}
},
"tags": {
"type": "array",
"items": {
"type": "string"
},
"description": "A list of 0 or more tags that are associated with this workbook definition"
},
"userId": {
"type": "string",
"description": "Unique user id of the specific user that owns this workbook."
},
"sourceResourceId": {
"type": "string",
"description": "Optional resourceId for a source resource."
}
}
}
|
|
I have updated mine to make PATCH operation of properties as not required. The PR is @ #2950 |
|
@ericc1103 thanks, your PR is for new preview api-version. The issue here is with 2015-05-01 version of workbooks. I replied to your email and clarified, please take a look. |
| "$ref": "#/parameters/ResourceNameParameter" | ||
| } | ||
| ], | ||
| "responses": { |
There was a problem hiding this comment.
add a default response to capture the error responses. The schema should match the ARm error contract schema. Example can be found in Batch RP swagger.
| "readOnly": true, | ||
| "description": "Azure resource Id" | ||
| }, | ||
| "name": { |
There was a problem hiding this comment.
this should also be readonly as it comes from the URL
| "description": "A ProactiveDetection configuration definition.", | ||
| "type": "object", | ||
| "x-ms-azure-resource": true, | ||
| "properties": { |
There was a problem hiding this comment.
"tags" ARM top level property missing? Is this a tracked or proxy resource? Seems tracked since it has a location property
| "description": "Properties that define a ProactiveDetection configuration.", | ||
| "type": "object", | ||
| "properties": { | ||
| "Name": { |
There was a problem hiding this comment.
how is this name different from the one outside of properties? If different, please call this something else like ruleName
| "readOnly": false, | ||
| "description": "The rule name" | ||
| }, | ||
| "Enabled": { |
There was a problem hiding this comment.
bools are typically not recommended. String enums generally work better.
| "readOnly": false, | ||
| "description": "A flag that indicates whether this rule is enabled by the user" | ||
| }, | ||
| "SendEmailsToSubscriptionOwners": { |
| "type": "string" | ||
| } | ||
| }, | ||
| "LastUpdatedTime": { |
There was a problem hiding this comment.
readonly should be true
| } | ||
| }, | ||
| "paths": { | ||
| "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Insights/components/{resourceName}/ProactiveDetectionConfigs": |
There was a problem hiding this comment.
Delete API missing. If this is a tracked resource type, patch also MUST be supported atleast for the update of tags but preferably to allow update of any patachable properties.
|
@aviled any thoughts on @ravbhatnagar 's comments? |
|
@aviled any updates? If I don't hear back in the next day, we'll be closing the PR and you can reopen it when you're back to it. Thanks! |
|
Closing due to inactivity, please reopen/comment when you're back to this. Thanks. |
6 participants
No description provided.