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

Linter incorrectly flags properties with description as "No description" #14699

Closed
matthchr opened this issue Jun 7, 2021 · 4 comments
Closed
Assignees

Comments

@matthchr
Copy link
Member

matthchr commented Jun 7, 2021

The linter flags fields that look like this as having no title or description:

    "ManagedClusterAgentPoolProfileProperties": {
      "properties": {
        "osDiskSizeGB": {
          "$ref": "#/definitions/ContainerServiceOSDisk"
        },
        "osDiskType": {
          "$ref": "#/definitions/OSDiskType"
        }

In reality, if you look at the definition of these refs:

    "OSDiskType": {
      "type": "string",
      "enum": [
        "Managed",
        "Ephemeral"
      ],
      "x-ms-enum": {
        "name": "OSDiskType",
        "modelAsString": true,
        "values": [
          {
            "value": "Managed",
            "description": "Azure replicates the operating system disk for a virtual machine to Azure storage to avoid data loss should the VM need to be relocated to another host. Since containers aren't designed to have local state persisted, this behavior offers limited value while providing some drawbacks, including slower node provisioning and higher read/write latency."
          },
          {
            "value": "Ephemeral",
            "description": "Ephemeral OS disks are stored only on the host machine, just like a temporary disk. This provides lower read/write latency, along with faster node scaling and cluster upgrades."
          }
        ]
      },
      "title": "The OS disk type to be used for machines in the agent pool.",
      "description": "The default is 'Ephemeral' if the VM supports it and has a cache disk larger than the requested OSDiskSizeGB. Otherwise, defaults to 'Managed'. May not be changed after creation. For more information see [Ephemeral OS](https://docs.microsoft.com/azure/aks/cluster-configuration#ephemeral-os)."
    },

and

"ContainerServiceOSDisk": {
      "type": "integer",
      "format": "int32",
      "maximum": 2048,
      "minimum": 0,
      "description": "OS Disk Size in GB to be used to specify the disk size for every machine in the master/agent pool. If you specify 0, it will apply the default osDisk size according to the vmSize specified."
    },

They have a title and description.

I do understand that many times the title or description of a complex type is not appropriate to put on the property where that type is used. But in the case of fields like ContainerServiceOSDisk, the ref seems more for "code reuse" in the Swagger and less about definition a new type (since the type in question is a primitive type and AFAIK will be rendered as such everywhere).

Thoughts on not flagging this with the linter?

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 7, 2021
@matthchr
Copy link
Member Author

matthchr commented Jun 7, 2021

You can see an example of this here: #14696

@leni-msft leni-msft removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 8, 2021
@leni-msft leni-msft removed their assignment Jun 8, 2021
@jianyexi
Copy link
Contributor

jianyexi commented Jul 6, 2021

good capture @matthchr , will fix it in Azure/azure-openapi-validator#246

@matthchr
Copy link
Member Author

@jianyexi looks like you can close this?

@jianyexi
Copy link
Contributor

@jianyexi Jianye Xi FTE looks like you can close this?

yes , fixed it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants