web: type error in sslState in HostNameSslState class #141

lmazuel · 2016-02-18T20:14:39Z

Hello,

I work in the MS Azure Python SDK team and I'm currently trying the Autorest generated version for Python,
I'm just trying to do a get a a website using webspace+name using this Python SDK generated from the Web Swagger file (rev. c81ae63) with the "Sites_GetSite method.
I got in result for hostNameSslStates

    "hostNameSslStates": [
      {
        "name": "mysite.azurewebsites.net",
        "sslState": 0,
        "ipBasedSslResult": null,
        "virtualIP": null,
        "thumbprint": null,
        "toUpdate": null,
        "toUpdateIpBasedSsl": null,
        "ipBasedSslState": 0,
        "hostType": 0
      },

Regarding the Swagger spec, "sslState" is an enum:

"sslState": {
          "description": "SSL type",
          "enum": [
            "Disabled",
            "SniEnabled",
            "IpBasedEnabled"
          ],
          "type": "string",
          "x-ms-enum": {
            "name": "SslState",
            "modelAsString": false
          }
        },

However, as you can see the sslState I got is 0, not a string.

It's the second bug (after #137) I open on the web file so far, is this possible to have at least a confirmation or state from the web team about this?
Thank you,

The text was updated successfully, but these errors were encountered:

amarzavery · 2016-02-19T19:47:30Z

@naveedaz - Can you please take a look into this issue?

amarzavery · 2016-02-19T22:21:59Z

@lmazuel
I have no clue why is the server sending you 0 instead of one of the valid values. This definitely is a bug on the server side. Either the spec needs to be updated or the server side needs a fix.

From code gen point of view, the extension "x-ms-enum" is documented over here.
Since "modelAsString" is set to false, sslState would be an enum in C# and Java. Node.js does not support enums. The generated node.js code will do the validation (throw if the value is not from the set of valid values) in this case.

I am not sure about Python. v3.4 has enums implemented and v2.7 does not.
@xingwu1 who implemented the python code gen would be in a better position to comment on the behavior in python.

amarzavery · 2016-02-19T22:26:45Z

@lmazuel - Update to my comment:
Sorry, I concluded too fast. Is this the response from the server or the deserialized response from the python code gen? If the server is sending the correct response (string for sslState) then Python codegen is doing something different (Returning index of the enumValue)..

lmazuel · 2016-02-19T22:30:14Z

This is the actual response from the server, I know it's important to check that this is not a language specific problem. No Python here :)
I cite Python for the record, to give context, but really it's the actual JSON from the server.

lmazuel · 2016-02-19T22:31:37Z

Either the spec needs to be updated or the server side needs a fix.

It's what I think too.

amarzavery · 2016-02-19T22:34:56Z

Thanks for confirming. Then this is a server side issue. @naveedaz from webapps team would be in a better position to comment.

amarzavery · 2016-02-24T20:32:22Z

@naveedaz - Any update on this?

naveedaz · 2016-02-24T23:47:35Z

From Microsoft.Web standpoint the response has enumerations as proper enumtypes like for SSLState: Disabled, SniEnabled, IpBasedEnabled etc.
However the requests are brokered from user to resource provider by Azure resource Manager, and the response from there seems to have 0,1,2 instead of the enum types.
@IlyaGrebnov Can you elaborate on why that is the case?

IlyaGrebnov · 2016-02-25T06:23:56Z

@naveedaz ARM doesn't know schema of individual resources, so maping "string" to int (and vise-versa) is not possible at our layer. Most likely issue is caused by Microsoft.Web RP or client tools/libs.

lmazuel · 2016-02-25T17:49:04Z

@naveedaz @IlyaGrebnov I can assure you that this it NOT a client tools/libs problem: the answer I got is direct from the REST API.

IlyaGrebnov · 2016-02-25T22:43:50Z

Yes, I believe this is know bug in Antares/Geomaster RP which serialize enums to numbers by default.

lmazuel · 2016-03-08T22:31:01Z

Any news on this one?

I made a screenshot of resources.azure.com to establish at 100% that this is not a Python issue

lmazuel · 2016-03-16T00:25:38Z

@IlyaGrebnov @naveedaz Is this possible to have a status on this one?
For now, it was "just" internally by me but now I have an open bug which related to this one (Azure/azure-sdk-for-python#544)

At least a status: we will fix it, we won't, you will need a new API version, there is an open issue there http://xxxx, etc.
Another guy in our team had to patch his code manually to do his //Build demo.
There is no workaround since the JSON exposed by the API uses an index from an enum we don't know (we have no guarantee that the enum order in the Swagger file is the one considered by the server).

Thank you for considering this.

IlyaGrebnov · 2016-03-16T22:45:55Z

@naveedaz could you please comment on this? ARM simple proxy response from resource provider. And I believe this is issue on 'GeoMester'.

devigned · 2016-05-02T17:40:50Z

@naveedaz are there any plan to correct this issue, or should we expect the API to continue to return the integer response?

If we are to expect the API to return the integer response, can you then ensure that any new enums will be added to the end of the list, thus preserving the correctness of the generated code? Any change to the order of the enumeration will be a breaking change to end users.

/cc @kamaljit for visibility.

amarzavery · 2016-10-05T21:03:49Z

@naveedaz - It has been open for more than 6 months now. Any ETA for this fix?

naveedaz · 2016-10-12T17:57:02Z

We are working on a fix targeted for next release ETA end of month. The fix will be api-version specific to prevent breaking existing clients.

jhendrixMSFT · 2016-10-12T18:02:00Z

Thanks for the update, will that fix include the issue mentioned in #137?

naveedaz · 2016-10-12T20:43:02Z

Yes. It will address that issue as well.

jhendrixMSFT · 2016-10-27T19:25:32Z

@naveedaz Is this still on track to be fixed by the end of this month?

naveedaz · 2016-10-27T19:35:02Z

@jhendrixMSFT The fix was checked in 10/13/2016. However, we hit some bumps with our deployment train and is now targeted for Nov 11 2016. Users will have to use api-version >= 2016-08-01. The latest composite swagger files have the updated api versions. https://github.com/Azure/azure-rest-api-specs/blob/master/arm-web/compositeWebAppClient.json.

naveedaz · 2016-11-17T01:00:17Z

FYI the fix is deployed. Users will have to use api-version >= 2016-08-01. The latest composite swagger files have the updated api versions. https://github.com/Azure/azure-rest-api-specs/blob/master/arm-web/compositeWebAppClient.json.

"hostNameSslStates": [
      {
        "name": "naveedalb.trafficmanager.net",
        "sslState": "Disabled",
        "ipBasedSslResult": null,
        "virtualIP": null,
        "thumbprint": null,
        "toUpdate": null,
        "toUpdateIpBasedSsl": null,
        "ipBasedSslState": "NotConfigured",
        "hostType": "Standard"
      },
      {
        "name": "naveedasrc.azurewebsites.net",
        "sslState": "Disabled",
        "ipBasedSslResult": null,
        "virtualIP": null,
        "thumbprint": null,
        "toUpdate": null,
        "toUpdateIpBasedSsl": null,
        "ipBasedSslState": "NotConfigured",
        "hostType": "Standard"
      },
      {
        "name": "naveedasrc.scm.azurewebsites.net",
        "sslState": "Disabled",
        "ipBasedSslResult": null,
        "virtualIP": null,
        "thumbprint": null,
        "toUpdate": null,
        "toUpdateIpBasedSsl": null,
        "ipBasedSslState": "NotConfigured",
        "hostType": "Repository"
      }
    ]

* Adding Microsoft.Web ResourceHealthMetadata APIs Adding the new Microsoft.Web ResourceHealthMetadata resource APIs. Includes subscription level list, resource group level list, site level list, and site level get * Fixing ResourceHealthMetadata operation descriptions and updating readOnly properties As per comments from dsgouda, making descriptions contain more information than the summary. Additionally marking certain properties as readOnly as they ought to be * Removing ResourceHealthMetadata x-ms-pageable readonly property to fix CI x-ms-pageable can't have any additional properties, so can't make it readOnly * Microsoft.Web ResourceHealthMetadata API removing text/json as a potential produces type As per dsgouda, ARM only support application json types, so removing the "text/json" produces type from all the APIs

lmazuel mentioned this issue Mar 24, 2016

Allows to parse integer as enum valid values Azure/autorest#879

Merged

naveedaz closed this as completed Nov 18, 2016

MarcelRaschke mentioned this issue Aug 20, 2022

[Snyk] Upgrade @octokit/rest from 15.18.3 to 19.0.3 devcode1981/azure-rest-api-specs#5

Open

github-actions bot mentioned this issue Apr 2, 2024

az/2.59.0 package update wolfi-dev/os#16155

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

web: type error in sslState in HostNameSslState class #141

web: type error in sslState in HostNameSslState class #141

lmazuel commented Feb 18, 2016

amarzavery commented Feb 19, 2016

amarzavery commented Feb 19, 2016

amarzavery commented Feb 19, 2016

lmazuel commented Feb 19, 2016

lmazuel commented Feb 19, 2016

amarzavery commented Feb 19, 2016

amarzavery commented Feb 24, 2016

naveedaz commented Feb 24, 2016

IlyaGrebnov commented Feb 25, 2016

lmazuel commented Feb 25, 2016

IlyaGrebnov commented Feb 25, 2016

lmazuel commented Mar 8, 2016

lmazuel commented Mar 16, 2016

IlyaGrebnov commented Mar 16, 2016

devigned commented May 2, 2016

amarzavery commented Oct 5, 2016

naveedaz commented Oct 12, 2016

jhendrixMSFT commented Oct 12, 2016

naveedaz commented Oct 12, 2016

jhendrixMSFT commented Oct 27, 2016

naveedaz commented Oct 27, 2016

naveedaz commented Nov 17, 2016 •

edited by devigned

web: type error in sslState in HostNameSslState class #141

web: type error in sslState in HostNameSslState class #141

Comments

lmazuel commented Feb 18, 2016

amarzavery commented Feb 19, 2016

amarzavery commented Feb 19, 2016

amarzavery commented Feb 19, 2016

lmazuel commented Feb 19, 2016

lmazuel commented Feb 19, 2016

amarzavery commented Feb 19, 2016

amarzavery commented Feb 24, 2016

naveedaz commented Feb 24, 2016

IlyaGrebnov commented Feb 25, 2016

lmazuel commented Feb 25, 2016

IlyaGrebnov commented Feb 25, 2016

lmazuel commented Mar 8, 2016

lmazuel commented Mar 16, 2016

IlyaGrebnov commented Mar 16, 2016

devigned commented May 2, 2016

amarzavery commented Oct 5, 2016

naveedaz commented Oct 12, 2016

jhendrixMSFT commented Oct 12, 2016

naveedaz commented Oct 12, 2016

jhendrixMSFT commented Oct 27, 2016

naveedaz commented Oct 27, 2016

naveedaz commented Nov 17, 2016 • edited by devigned

naveedaz commented Nov 17, 2016 •

edited by devigned