-
Notifications
You must be signed in to change notification settings - Fork 5.7k
updating location capabilities for 2023-05-01-preview #25539
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,8 @@ | |
| "supportedElasticPoolEditions", | ||
| "supportedManagedInstanceVersions", | ||
| "supportedInstancePoolEditions", | ||
| "supportedManagedInstanceEditions" | ||
| "supportedManagedInstanceEditions", | ||
| "supportedJobAgentVersions" | ||
| ], | ||
| "x-ms-enum": { | ||
| "name": "CapabilityGroup", | ||
|
|
@@ -579,6 +580,119 @@ | |
| } | ||
| } | ||
| }, | ||
| "JobAgentEditionCapability": { | ||
| "description": "The job agent edition capability.", | ||
| "type": "object", | ||
| "properties": { | ||
| "name": { | ||
| "description": "The job agent edition name.", | ||
| "type": "string", | ||
| "readOnly": true | ||
| }, | ||
| "supportedServiceLevelObjectives": { | ||
| "description": "The list of supported service level objectives for the edition.", | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/definitions/JobAgentServiceLevelObjectiveCapability" | ||
| }, | ||
| "readOnly": true, | ||
| "x-ms-identifiers": [] | ||
| }, | ||
| "status": { | ||
| "description": "The status of the capability.", | ||
| "enum": [ | ||
| "Visible", | ||
| "Available", | ||
| "Default", | ||
| "Disabled" | ||
| ], | ||
| "type": "string", | ||
| "readOnly": true, | ||
| "x-ms-enum": { | ||
| "name": "CapabilityStatus", | ||
| "modelAsString": false | ||
| } | ||
| }, | ||
| "reason": { | ||
| "description": "The reason for the capability not being available.", | ||
| "type": "string" | ||
| } | ||
| } | ||
| }, | ||
| "JobAgentServiceLevelObjectiveCapability": { | ||
| "description": "The job agent service level objective capability.", | ||
| "type": "object", | ||
| "properties": { | ||
| "name": { | ||
| "description": "The service objective name.", | ||
| "type": "string", | ||
| "readOnly": true | ||
| }, | ||
| "sku": { | ||
| "$ref": "../../../common/v1/types.json#/definitions/Sku", | ||
| "description": "The sku.", | ||
| "readOnly": true | ||
| }, | ||
| "status": { | ||
| "description": "The status of the capability.", | ||
| "enum": [ | ||
| "Visible", | ||
| "Available", | ||
| "Default", | ||
| "Disabled" | ||
| ], | ||
| "type": "string", | ||
| "readOnly": true, | ||
| "x-ms-enum": { | ||
| "name": "CapabilityStatus", | ||
| "modelAsString": false | ||
| } | ||
| }, | ||
| "reason": { | ||
| "description": "The reason for the capability not being available.", | ||
| "type": "string" | ||
| } | ||
| } | ||
| }, | ||
| "JobAgentVersionCapability": { | ||
| "description": "The job agent version capability.", | ||
| "type": "object", | ||
| "properties": { | ||
| "name": { | ||
| "description": "The job agent version name.", | ||
| "type": "string", | ||
| "readOnly": true | ||
| }, | ||
| "supportedEditions": { | ||
| "description": "The list of supported editions.", | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/definitions/JobAgentEditionCapability" | ||
| }, | ||
| "readOnly": true, | ||
| "x-ms-identifiers": [] | ||
| }, | ||
| "status": { | ||
| "description": "The status of the capability.", | ||
| "enum": [ | ||
| "Visible", | ||
| "Available", | ||
| "Default", | ||
| "Disabled" | ||
| ], | ||
| "type": "string", | ||
| "readOnly": true, | ||
| "x-ms-enum": { | ||
| "name": "CapabilityStatus", | ||
| "modelAsString": false | ||
| } | ||
| }, | ||
| "reason": { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to only apply for certain status values. What is it set to for other status values? Empty string? Something else? Consider making this more general, e.g. "details" or "detailedStatus", so it makes sense for any/all status values. Same comment applies for 2 other "reason" properties.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As with the name property we are trying to be consistent with all of the other location capabilities that have been defined this way for several API versions. |
||
| "description": "The reason for the capability not being available.", | ||
| "type": "string" | ||
| } | ||
| } | ||
| }, | ||
| "LicenseTypeCapability": { | ||
| "description": "The license type capability", | ||
| "type": "object", | ||
|
|
@@ -634,6 +748,15 @@ | |
| }, | ||
| "readOnly": true | ||
| }, | ||
| "supportedJobAgentVersions": { | ||
| "description": "The list of supported job agent versions.", | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/definitions/JobAgentVersionCapability" | ||
| }, | ||
| "readOnly": true, | ||
| "x-ms-identifiers": [] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| }, | ||
| "status": { | ||
| "description": "The status of the capability.", | ||
| "enum": [ | ||
|
|
||
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.
Don't use the ARM top-level property names (name, type, id, systemData) for properties within the property bag. Same comment applies for 2 other "name" properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how ElasticPoolEditionCapability, InstancePoolVcoresCapability, and other objects in this file are structured. This is the format they have had for several API versions now and we are seeking consistency with them.
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.
@mentat9 please advise on whether this is acceptable.
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.
There are some apis or parts of apis, that don't follow this guidance, but in general you want to avoid it in properties, because if you flatten away the properties in the SDK code generation, which is good for usability, it becomes a duplicate 'name' conflict. [Not to mention a point of potentially confusing people as to why there are so many names.]
Uh oh!
There was an error while loading. Please reload this page.
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 think in this context its acceptable usage and okay to keep consistency with published APIs.