Skip to content
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

[Fix S360 Broken Issues][HDInsight]Fix s360 issues batch3 #12612

Merged
merged 16 commits into from
Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion custom-words.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2069,4 +2069,7 @@ Metastores
maintenancewindows
appendpos
appendblock
Exprired
Exprired
azureasyncoperations
saskey
vmsize
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@
"format": "int32",
"description": "The public port to connect to."
},
"privateIPAddress": {
"type": "string",
"description": "The private ip address of the endpoint."
},
"subDomainSuffix": {
"type": "string",
"description": "The subdomain suffix of the application."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,51 @@
},
"x-ms-long-running-operation": true
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.HDInsight/clusters/{clusterName}/azureasyncoperations/{operationId}": {
"get": {
"tags": [
"Clusters"
],
"operationId": "Clusters_GetAzureAsyncOperationStatus",
"description": "The the async operation status.",
"x-ms-examples": {
"Get Async Operation Status of Creating Cluster": {
"$ref": "./examples/GetClusterCreatingAsyncOperationStatus.json"
}
},
"parameters": [
{
"$ref": "#/parameters/SubscriptionIdParameter"
},
{
"$ref": "#/parameters/ResourceGroupNameParameter"
},
{
"$ref": "#/parameters/ClusterNameParameter"
},
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/OperationIdParameter"
}
],
"responses": {
"default": {
"description": "Error response describing why the operation failed.",
"schema": {
"$ref": "./operations.json#/definitions/ErrorResponse"
}
},
"200": {
"description": "OK response definition.",
"schema": {
"$ref": "#/definitions/AsyncOperationResult"
}
}
}
}
}
},
"definitions": {
Expand Down Expand Up @@ -930,6 +975,10 @@
"format": "int32",
"description": "The instance count of the cluster."
},
"VMGroupName": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this property name is not CamelCase, but in order to fix s360 issues, I need to change this to keep consistent with backend.
Our service will GA a new api version in April, in the new api version, I will fix this issue. Now I can create a work item or github issue to track this.
If it is ok, I hope this can check in. @JeffreyRichter You opinion is important for us, I will follow your suggestion. How do you like?

Copy link
Member

@JeffreyRichter JeffreyRichter Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are approving breaking changes until the end of January 2021 so that the swager matches the service. So, if the service uses VMGroupName, then the service is wrong (it should have been vmGroupName) but it is too late now and the service must forever be wrong. And so the swagger must also be wrong.

"type": "string",
"description": "The name of the virtual machine group."
},
"autoscale": {
"$ref": "#/definitions/Autoscale",
"x-ms-client-name": "autoscaleConfiguration",
Expand Down Expand Up @@ -1009,6 +1058,14 @@
"msiResourceId": {
"type": "string",
"description": "The managed identity (MSI) that is allowed to access the storage account, only to be specified for Azure Data Lake Storage Gen 2."
},
"saskey": {
Copy link
Member Author

@aim-for-better aim-for-better Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same with VMGroupName, per my understanding, sasKey , K is Capital will be better, but CI does not report this is not CamelCase, there is just a unknown word issue.

I will create a work item to track this.

"type": "string",
"description": "The shared access signature key."
},
"fileshare": {
Copy link
Member Author

@aim-for-better aim-for-better Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same with saskey, per my understanding, fileShare will be better, but there is not CI error related with this property name. Now I want to keep consistent with backend, I will create a work item to track this, and will fix this issue in our new API version.

"type": "string",
"description": "The file share name."
}
},
"description": "The storage Account."
Expand Down Expand Up @@ -1733,7 +1790,7 @@
},
"description": "Gateway settings."
},
"OperationResource": {
"AsyncOperationResult": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to rename this model? This renaming causes SDK breaking changes.
This model is not used at all in the previous version of this swagger, therefore could we just keep this name unchanged?

Copy link
Member Author

@aim-for-better aim-for-better Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to rename this model? This renaming causes SDK breaking changes.
This model is not used at all in the previous version of this swagger, therefore could we just keep this name unchanged?

Hi @ArcturusZhang , Thanks for your comment.
I think the new name is more meaningful. As you said, the class "OperationResource" is never used. I think this breaking change is ok, no customer will be impacted.

I know this PR will cause breaking change but we have to fix S360 issues.I just submitted the sdk release requirement about one week ago. Now we do not have the requirement for release sdk based this PR. Thank you very much~

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ArcturusZhang , Is the reply accpetable for you? Besides this class name, there are other changes will cause breaking change too. This PR is fixing the S360 issues which will cause breaking changes in Sdks level.

"properties": {
"status": {
"type": "string",
Expand Down Expand Up @@ -1845,6 +1902,14 @@
"required": true,
"type": "string",
"description": "The HDInsight client API Version."
},
"OperationIdParameter": {
"name": "operationId",
"in": "path",
"required": true,
"type": "string",
"description": "The long running operation id.",
"x-ms-parameter-location": "method"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"parameters": {
"clusterName": "cluster1",
"resourceGroupName": "rg1",
"api-version": "2018-06-01-preview",
"subscriptionId": "subid",
"operationId": "CF938302-6B4D-44A0-A6D2-C0D67E847AEC"
},
"responses": {
"200": {
"body": {
"status": "InProgress"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
"responses": {
"200": {
"body": {
"workspaceId": "id",
"primaryKey": "key"
"clusterMonitoringEnabled": true,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change in example file is in order to keep consistent with the change in swagger spec file.

"workspaceId": "id"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
{
"friendlyName": "1.6",
"displayName": "HdInsight version 1.6.1.0.335536",
"isDefault": "false"
"isDefault": false
},
{
"friendlyName": "2.1",
"displayName": "Version 2.1.9.406.1221105",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "1.3",
"Hadoop": "1.2.0"
Expand All @@ -27,7 +27,7 @@
{
"friendlyName": "3.0",
"displayName": "Version 3.0.6.989.2441725",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.0",
"Hadoop": "2.2.0"
Expand All @@ -36,7 +36,7 @@
{
"friendlyName": "3.1",
"displayName": "Version 3.1.4.989.2441725",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.1.7",
"Hadoop": "2.4.0",
Expand All @@ -46,7 +46,7 @@
{
"friendlyName": "3.2",
"displayName": "Version 3.2.7.989.2441725",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.2",
"Hadoop": "2.6.0",
Expand All @@ -57,7 +57,7 @@
{
"friendlyName": "3.3",
"displayName": "Version 3.3.0.989.2441725",
"isDefault": "true",
"isDefault": true,
"componentVersions": {
"HDP": "2.3",
"Hadoop": "2.7.0",
Expand All @@ -72,7 +72,7 @@
{
"friendlyName": "3.2",
"displayName": "Version 3.2.1000.0.8840373",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.2",
"Hadoop": "2.6.0",
Expand All @@ -83,7 +83,7 @@
{
"friendlyName": "3.3",
"displayName": "Version 3.3.1000.0.9776961",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.3",
"Hadoop": "2.7.0",
Expand All @@ -95,7 +95,7 @@
{
"friendlyName": "3.4",
"displayName": "Version 3.4.1000.0.9719475",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.4",
"Hadoop": "2.7.1",
Expand All @@ -108,7 +108,7 @@
{
"friendlyName": "3.5",
"displayName": "Version 3.5.1000.0.9732704",
"isDefault": "true",
"isDefault": true,
"componentVersions": {
"HDP": "2.5",
"Hadoop": "2.7.3",
Expand All @@ -122,7 +122,7 @@
{
"friendlyName": "3.6",
"displayName": "Version 3.6.1000.0.9503998",
"isDefault": "false",
"isDefault": false,
"componentVersions": {
"HDP": "2.6",
"Spark": "2.1.0"
Expand All @@ -131,7 +131,7 @@
{
"friendlyName": "99.152",
"displayName": "Version 99.152.1000.0.6943836",
"isDefault": "false"
"isDefault": false
}
]
}
Expand Down Expand Up @@ -184,7 +184,7 @@
]
}
},
"vmSizes": {
"vmsizes": {
Copy link
Member Author

@aim-for-better aim-for-better Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change in example file is in order to keep consistent with the change in swagger spec file.

"paas": {
"available": [
"A5",
Expand Down Expand Up @@ -242,7 +242,7 @@
]
}
},
"vmSize_filters": [
"vmsize_filters": [
Copy link
Member Author

@aim-for-better aim-for-better Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change in example file is in order to keep consistent with the change in swagger spec file.

{
"FilterMode": "Exclude",
"Regions": [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"parameters": {
"location": "East US 2",
"api-version": "2018-06-01-preview",
"subscriptionId": "subid",
"operationId": "8a0348f4-8a85-4ec2-abe0-03b26104a9a0-0"
},
"responses": {
"200": {
"body": {
"status": "Succeeded"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@
"200": {
"description": "OK. The request has succeeded.",
"schema": {
"$ref": "#/definitions/Extension"
"$ref": "#/definitions/ClusterMonitoringResponse"
}
}
}
Expand Down Expand Up @@ -318,7 +318,7 @@
},
"definitions": {
"Extension": {
"description": "Cluster monitoring extensions",
"description": "Cluster monitoring extensions.",
"properties": {
"workspaceId": {
"description": "The workspace ID for the cluster monitoring extension.",
Expand All @@ -331,27 +331,27 @@
}
},
"ClusterMonitoringResponse": {
"description": "The Operations Management Suite (OMS) status response",
"description": "The cluster monitoring status response.",
"properties": {
"clusterMonitoringEnabled": {
"description": "The status of the Operations Management Suite (OMS) on the HDInsight cluster.",
"description": "The status of the monitor on the HDInsight cluster.",
"type": "boolean"
},
"workspaceId": {
"description": "The workspace ID of the Operations Management Suite (OMS) on the HDInsight cluster.",
"description": "The workspace ID of the monitor on the HDInsight cluster.",
"type": "string"
}
}
},
"ClusterMonitoringRequest": {
"description": "The Operations Management Suite (OMS) parameters.",
"description": "The cluster monitor parameters.",
"properties": {
"workspaceId": {
"description": "The Operations Management Suite (OMS) workspace ID.",
"description": "The cluster monitor workspace ID.",
"type": "string"
},
"primaryKey": {
"description": "The Operations Management Suite (OMS) workspace key.",
"description": "The cluster monitor workspace key.",
"type": "string"
}
}
Expand Down
Loading