[HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum #6665

aim-for-better · 2019-07-18T10:03:02Z

[HDInsight]

Add capabilities to stable/2018-06-01-preview/locations.json.
Modify applicationType and accessModes to enum in both stable and preview api version.
In order to be consistent with response from RP side, Add suppression DefinitionsPropertiesNamesCamelCase

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

I have reviewed the documentation for the workflow.
Validation tools were run on swagger spec(s) and have all been fixed in this PR.
The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
adding/removing APIs.
adding/removing properties.
adding/removing API-version.
adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Please follow the link to find more details on API review process.

openapi-sdkautomation · 2019-07-18T10:03:06Z

SDK Automation [Logs] (Generated from a22cf02, Iteration 12)

Go: Azure/azure-sdk-for-go [Logs] [Diff]

preview/hdinsight/mgmt/2015-03-01-preview [Logs]
preview/hdinsight/mgmt/2018-06-01-preview [Logs]

Java: Azure/azure-sdk-for-java [Logs] [Diff]

hdinsight/resource-manager/v2015_03_01_preview [Logs]
hdinsight/resource-manager/v2018_06_01_preview [Logs]

Python: Azure/azure-sdk-for-python [Logs] [Diff]

azure-mgmt-hdinsight [Logs]

JavaScript: Azure/azure-sdk-for-js [Logs] [Diff]

@azure/arm-hdinsight [Logs]

AutorestCI · 2019-07-18T10:04:17Z

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#6082

AutorestCI · 2019-07-18T10:05:54Z

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#5369

azuresdkci · 2019-07-18T10:05:58Z

Can one of the admins verify this patch?

AutorestCI · 2019-07-18T10:06:58Z

Automation for azure-sdk-for-java

A 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:
Azure/azure-sdk-for-java#3285

anuchandy · 2019-07-18T15:17:26Z

...n/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/applications.json

+            "x-ms-enum": {
+              "name": "ApplicationHttpsEndpointAccessMode",
+              "modelAsString": true
+            }


This is a breaking change. Also a bit confused why do we have preview-api-version under stable directory?

This is a breaking change. Also a bit confused why do we have preview-api-version under stable directory?

@anuchandy This is intentional. It’s a little confusing, but per rule #5 here, whether the swagger spec is in the preview or stable folder “is not a direct analog for whether or not an API Version has the ‘-preview’ suffix or not. SDKs that are generated from 'preview' folder items should indicate to their customers in the most idiomatic way that breaking changes may still be coming.”

Basically the short version is that HDInsight is a GA service that still has an API with a -preview tag. Somebody made the decision several years ago to release stable versions of Hydra/Hyak SDKs for .NET, JavaScript, etc. that consumed this the -preview API. We are now releasing stable swagger-based SDKs so we can at least finally deprecate the Hydra/Hyak SDKs. As a result, we are making a commitment that we will not make breaking changes to this swagger spec version so we can GA some swagger-based SDKs.

@aim-for-better to address the "is this a breaking change" question - as per my comment on the other enum change, please confirm that WebPage is the only actual value the server can accept and return, and I think we're good.

anuchandy · 2019-07-18T15:18:36Z

...n/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/applications.json

+          "x-ms-enum": {
+            "name": "ApplicationType",
+            "modelAsString": true
+          }


breaking change.

breaking change.

Hi anuchandy, Why do you think it is a breaking change?
I have tested the generated .NET SDK based this swagger spec. In .NET SDK, it is not a breaking change. Because the modelAsString is true, it will change it to enum in .NET SDK
Can you give me a example that confirm that it will bring up a breaking change.

IMO this is only a breaking change if server can return a value (or allow setting a value) that isn't specified in this list. Otherwise, I don't see a problem. @aim-for-better please confirm.

IMO this is only a breaking change if server can return a value (or allow setting a value) that isn't specified in this list. Otherwise, I don't see a problem. @aim-for-better please confirm.

@antmarti-microsoft Thanks for you opinion. Now Rp server side only support these two types. So I think there is no problem.

@aim-for-better - the scope of breaking change is different for different languages, for example in java this cause generating a model that extends from ExpandableStringEnum with one value WebPage, like this. I agree that this is not a breaking change in API Level.

@aim-for-better - the scope of breaking change is different for different languages, for example in java this cause generating a model that extends from ExpandableStringEnum with one value WebPage, like this. I agree that this is not a breaking change in API Level.

Hi @anuchandy I have tested the java code, I think you are right. However the PR has been merged. So I will pull a new request to fix this breaking change.

anuchandy · 2019-07-18T15:20:56Z

...tion/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/locations.json

+          "$ref": "#/definitions/QuotaCapability"
+        }
+      }
+    },


I guess all properties of CapabilitiesResult model and the model it composes should be readonly, could you double check?

I guess all properties of CapabilitiesResult model and the model it composes should be readonly, could you double check?

Hi anuchandy, I have added the readonly parameter

.../hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/applications.json

anthony-c-martin · 2019-07-24T14:26:47Z

.../hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/applications.json

+        },
+        "subDomainSuffix": {
+          "type": "string",
+          "description": "The subDomainSuffix of the application and can not greater than 3 characters."


Please improve the readability of these descriptions (correct grammar, use words instead of writing subDomainSuffix). Note you can use the "maxLength" property to enforce the 3 character restriction.

Please improve the readability of these descriptions (correct grammar, use words instead of writing subDomainSuffix). Note you can use the "maxLength" property to enforce the 3 character restriction.

Updated. Remove "can not greater than 3 characters" and don't use maxLength

anthony-c-martin · 2019-07-24T14:29:31Z

.../hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/applications.json

+        },
+        "disableGatewayAuth": {
+          "type": "boolean",
+          "description": "The value indicates whether to disable GatewayAuth."


What about something simpler like "Disable gateway authentication"

What about something simpler like "Disable gateway authentication"

Updated

anthony-c-martin · 2019-07-24T14:32:24Z

...n/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/applications.json

+          "x-ms-enum": {
+            "name": "ApplicationType",
+            "modelAsString": true
+          }


IMO this is only a breaking change if server can return a value (or allow setting a value) that isn't specified in this list. Otherwise, I don't see a problem. @aim-for-better please confirm.

anthony-c-martin · 2019-07-24T14:33:39Z

...n/hdinsight/resource-manager/Microsoft.HDInsight/stable/2018-06-01-preview/applications.json

+            "x-ms-enum": {
+              "name": "ApplicationHttpsEndpointAccessMode",
+              "modelAsString": true
+            }


@aim-for-better to address the "is this a breaking change" question - as per my comment on the other enum change, please confirm that WebPage is the only actual value the server can accept and return, and I think we're good.

anthony-c-martin · 2019-07-24T14:34:23Z

...ion/hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/locations.json

    },
    "QuotaCapability": {
      "description": "The regional quota capability.",
      "properties": {
+        "cores_used": {


Please ensure you address this naming in your next API version

Please ensure you address this naming in your next API version

Yes. I have created a issue #6724

anthony-c-martin · 2019-07-24T14:34:32Z

...ion/hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/locations.json

+          "type": "integer",
+          "format": "int64"
+        },
+        "max_cores_allowed": {


Please ensure you address this naming in your next API version

Please ensure you address this naming in your next API version

Yes. I have created a issue #6724

anthony-c-martin · 2019-07-24T14:38:05Z

specification/hdinsight/resource-manager/readme.md

     where:
       - $.definitions.CapabilitiesResult.properties.vmSize_filters
       - $.definitions.RegionalQuotaCapability.properties.cores_available
       - $.definitions.RegionalQuotaCapability.properties.cores_used
       - $.definitions.RegionalQuotaCapability.properties.region_name
+       - $.definitions.QuotaCapability.properties.cores_used
+       - $.definitions.QuotaCapability.properties.max_cores_allowed
+       - $.definitions.VmSizeCompatibilityFilter.properties.ClusterVersions


What's the problem with $.definitions.VmSizeCompatibilityFilter.properties.ClusterVersions and all the other $.definitions.VmSizeCompatibilityFilter.properties.* property names. The casing looks fine - do they need to be added to this suppression?

What's the problem with $.definitions.VmSizeCompatibilityFilter.properties.ClusterVersions and all the other $.definitions.VmSizeCompatibilityFilter.properties.* property names. The casing looks fine - do they need to be added to this suppression?

@antmarti-microsoft In fact swagger violation tools require lowerCamelCase. So the recommended usage is clusterVersions etc. This is why we need add this suppression.

…Mode to Enum. In order to be consistent with the response from RP side, add suppression for DefinitionsPropertiesNamesCamelCase

nschonni · 2019-07-25T19:20:32Z

...ion/hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/locations.json

+        "cores_used": {
+          "description": "The number of cores used in the subscription.",
+          "type": "integer",
+          "format": "int64"


Would int32 make more sense size-wise?

Would int32 make more sense size-wise?

No, server data type is long

I don't envy someone with a over a 2 billion core bill 😆

…ionType to enum, this will bring breaking change in java sdk. Solution: remove the two enum, change it to string type

…pe to enum, this will bring breaking change in java sdk. Solution: remove the two enum, change it to string type (#6785)

AutorestCI mentioned this pull request Jul 18, 2019

[AutoPR hdinsight/resource-manager] [HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum Azure/azure-sdk-for-python#6402

Merged

azuresdkci assigned anuchandy Jul 18, 2019

azuresdkci requested a review from anuchandy July 18, 2019 10:04

AutorestCI mentioned this pull request Jul 18, 2019

[AutoPR hdinsight/resource-manager] [HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum Azure/azure-sdk-for-go#5291

Merged

AutorestCI mentioned this pull request Jul 18, 2019

[AutoPR hdinsight/resource-manager] [HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum Azure/azure-sdk-for-java#4465

Closed

aim-for-better force-pushed the AddCapabilitiesModifyApplication branch 3 times, most recently from ecb5ee7 to f8c3994 Compare July 18, 2019 13:09

anuchandy reviewed Jul 18, 2019

View reviewed changes

anuchandy added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jul 18, 2019

KrisBash added the ARMReviewInProgress label Jul 18, 2019

aim-for-better force-pushed the AddCapabilitiesModifyApplication branch 2 times, most recently from cef6f97 to 1a06967 Compare July 19, 2019 03:06

idear1203 added the HDInsight label Jul 24, 2019

anthony-c-martin reviewed Jul 24, 2019

View reviewed changes

.../hdinsight/resource-manager/Microsoft.HDInsight/preview/2015-03-01-preview/applications.json Show resolved Hide resolved

anthony-c-martin requested changes Jul 24, 2019

View reviewed changes

Add Capabilities to stable version, Modify ApplicationType and Access…

a22cf02

…Mode to Enum. In order to be consistent with the response from RP side, add suppression for DefinitionsPropertiesNamesCamelCase

aim-for-better force-pushed the AddCapabilitiesModifyApplication branch from 1a06967 to a22cf02 Compare July 25, 2019 02:32

nschonni reviewed Jul 25, 2019

View reviewed changes

anthony-c-martin approved these changes Jul 26, 2019

View reviewed changes

anthony-c-martin added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMReviewInProgress WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jul 26, 2019

anuchandy added the potential-sdk-breaking-change label Jul 29, 2019

anuchandy approved these changes Jul 29, 2019

View reviewed changes

anuchandy merged commit c3a81d4 into Azure:master Jul 29, 2019

aim-for-better mentioned this pull request Jul 30, 2019

[HDInsight] Change accessModes and applicationType from enum to string #6785

Merged

5 tasks

anuchandy pushed a commit that referenced this pull request Jul 31, 2019

Problem: in previous pr #6665 we modify accessModes and applicationTy…

b9db593

…pe to enum, this will bring breaking change in java sdk. Solution: remove the two enum, change it to string type (#6785)

aim-for-better mentioned this pull request Aug 13, 2019

[HDInsight] Add three new features GetCapabilites, AutoScale, GetBillingSpec Azure/azure-sdk-for-net#7254

Merged

anuchandy mentioned this pull request Aug 19, 2019

HDInsignt 2018-06-01-preview is under the stable folder #6752

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum #6665

[HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum #6665

aim-for-better commented Jul 18, 2019 •

edited

Loading

openapi-sdkautomation bot commented Jul 18, 2019 •

edited

Loading

AutorestCI commented Jul 18, 2019 •

edited

Loading

AutorestCI commented Jul 18, 2019 •

edited

Loading

azuresdkci commented Jul 18, 2019

AutorestCI commented Jul 18, 2019 •

edited

Loading

anuchandy Jul 18, 2019

aim-for-better Jul 19, 2019

anthony-c-martin Jul 24, 2019

anuchandy Jul 18, 2019

aim-for-better Jul 19, 2019

anthony-c-martin Jul 24, 2019

aim-for-better Jul 25, 2019

anuchandy Jul 29, 2019

aim-for-better Jul 30, 2019

anuchandy Jul 18, 2019

aim-for-better Jul 19, 2019

anthony-c-martin Jul 24, 2019

aim-for-better Jul 25, 2019

anthony-c-martin Jul 24, 2019

aim-for-better Jul 25, 2019

anthony-c-martin Jul 24, 2019

anthony-c-martin Jul 24, 2019

anthony-c-martin Jul 24, 2019

aim-for-better Jul 25, 2019

anthony-c-martin Jul 24, 2019

aim-for-better Jul 25, 2019

anthony-c-martin Jul 24, 2019

aim-for-better Jul 25, 2019 •

edited

Loading

nschonni Jul 25, 2019

aim-for-better Jul 26, 2019

nschonni Jul 26, 2019

[HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum #6665

[HDInsight] Add Capabilities to Stable API version,Modify ApplicationType,AccessModes to enum #6665

Conversation

aim-for-better commented Jul 18, 2019 • edited Loading

Latest improvements:

Contribution checklist:

ARM API Review Checklist

openapi-sdkautomation bot commented Jul 18, 2019 • edited Loading

SDK Automation [Logs] (Generated from a22cf02, Iteration 12)

AutorestCI commented Jul 18, 2019 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Jul 18, 2019 • edited Loading

Automation for azure-sdk-for-go

azuresdkci commented Jul 18, 2019

AutorestCI commented Jul 18, 2019 • edited Loading

Automation for azure-sdk-for-java

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aim-for-better Jul 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aim-for-better commented Jul 18, 2019 •

edited

Loading

openapi-sdkautomation bot commented Jul 18, 2019 •

edited

Loading

AutorestCI commented Jul 18, 2019 •

edited

Loading

AutorestCI commented Jul 18, 2019 •

edited

Loading

AutorestCI commented Jul 18, 2019 •

edited

Loading

aim-for-better Jul 25, 2019 •

edited

Loading