Docstrings used to concatenate title + description, now use only description

[dpwatrous]

I'm not sure if this was an intentional behavior change or a regression, but historically we (Azure Batch) have used both the title and description fields on properties for descriptive purposes, and relied on the fact that they would be displayed together in docstrings.

The new behavior leads to some very confusing docstrings. I'm trying to determine if we should switch to using the description field only, or if this is simply a bug. (Either way it does look like a breaking change). For more context, see the discussion on this PR about the usage of title vs description.

Here is an example of the old vs new behavior:

Swagger:

"results": {
  "type": "string",
  "title": "The final values of all variables used in the evaluation of the autoscale formula.",
  "description": "Each variable value is returned in the form $variable=value, and variables are separated by semicolons."
}

Old docstring behavior:

:param results: The final values of all variables used in the evaluation of the autoscale formula. Each variable value is returned in the form $variable=value, and variables are

New docstring behavior:

:param results: Each variable value is returned in the form $variable=value, and variables are separated by semicolons.

Note: I'm not sure when this behavior changed, but I'm seeing it both when using the latest autorest extensions, and when specifying version 4.0.73 of the Python extension, and 3.0.6351 of autorest core.

[iscai-msft]

@timotheeguerin do you know about any differences in core / m4 regarding supporting title?

[timotheeguerin]

I think m4 will report the title as summary and description as description

[dpwatrous]

Just FYI (I know this really isn't the right place for this), this is an issue in Java as well:

JavaDoc with title and description

Swagger

"version": {
  "type": "string",
  "title": "The version of the application to deploy. If omitted, the default version is deployed.",
  "description": "If this is omitted on a Pool, and no default version is specified for this application, the request fails with the error code InvalidApplicationPackageReferences and HTTP status code 409. If this is omitted on a Task, and no default version is specified for this application, the Task fails with a pre-processing error."
}

JavaDoc

/**
 * Set if this is omitted on a Pool, and no default version is specified for this application, the request fails with the error code InvalidApplicationPackageReferences and HTTP status code 409. If this is omitted on a Task, and no default version is specified for this application, the Task fails with a pre-processing error.

JavaDoc with only description

Swagger

"version": {
  "type": "string",
  "description": "Set the version of the application to deploy. If omitted, the default version is deployed. If this is omitted on a Pool, and no default version is specified for this application, the request fails with the error code InvalidApplicationPackageReferences and HTTP status code 409. If this is omitted on a Task, and no default version is specified for this application, the Task fails with a pre-processing error."
}

JavaDoc

/**
 * Set the version of the application to deploy. If omitted, the default version is deployed. If this is omitted on a Pool, and no default version is specified for this application, the request fails with the error code InvalidApplicationPackageReferences and HTTP status code 409. If this is omitted on a Task, and no default version is specified for this application, the Task fails with a pre-processing error.

I'm very much leaning toward preferring descriptions rather than splitting things between title and description like the examples above just to match common usage.

[iscai-msft]

oh interesting, thanks @timotheeguerin ! @dpwatrous looking at the versions you sent and the code in autorest.python, I don't think we've supported this for as long as I've worked on autorest (version 4.x is the previous version of autorest, we're currently on version 5.x). I'm going to bring in my manager @lmazuel to see if this is behavior that we want to add.

[dpwatrous]

Sounds good - feel free to message me on teams as well if that's easier (dawatrou MS alias). I believe @paterasMSFT may have already talked to @lmazuel a bit about this, but that was before we realized the scope went beyond just Python.

I don't really have a huge preference here, just looking to get some agreement on how we should be using these properties.

[matthchr]

I have a preference: (that title is honored). If we're not going to do something with it (which is fair enough if that's the policy) then IMO there should be a linter that warns against using it (as @dpwatrous points out not many teams use it but some do in some places). I believe there is a linter now but the linter says you must specify title or description (or both) which is obviously in conflict with the reality today that some generators ignore title.

Whatever the accepted behavior is determined to be, there probably should be some tests encoded covering it (I thought I had added some when I wrote the feature but they've probably been lost by the wayside in a lot of the languages over the years as the generators were rewritten).

[lmazuel]

Swagger expects "summary" and "description". Should Batch Swagger be fixed to do that in the first place?

I'd be happy to fix linter and all if necessary.

@iscai-msft let's make sure we do summary/description well. If we were to support "tittle" as alias of "summary" then I would expect M4 to do it for us anyway.

[dpwatrous]

@lmazuel The biggest area where I'm seeing issues is on properties, where summary isn't allowed, but title and description are.

If we do support both, I think it would be good to be explicit about whether they are intended to be displayed together in all cases (like they are in the docs site, and used to be for Python). Otherwise it can lead to phrasing which works well in one context but not another (see the Java example above which assumes it can add "Get" or "Set" in front of descriptions.

[matthchr]

As @dpwatrous said, the question is for title + description in the definitions section of the Swagger @lmazuel. AutoRest already does the right thing w.r.t. summary and description on operations.

[iscai-msft]

Hey @dpwatrous @matthchr , starting to look into this. I have one question though: are you guys wanting to keep generating your SDK with v4 autorest.python, or are you guys looking to upgrade to the track 2 generator (latest v5 autorest.python) to generate your SDKs

[dpwatrous]

For Batch we're going to be stuck on v4 for a little while longer for the data plane, although the management plane uses v5 already.

Since OpenAPI 3.0 is a superset of JSON schema, I've been trying to find something in the official spec that speaks to usage of title vs description. It's fairly ambiguous, but there is an example which uses both under section 7.7.1.1 which to me seems to point to title being more like a friendly label than a short summary.

Below is a simplified version of the example from the spec, since other things like references aren't relevant to this discussion:

"properties": {
    "enabled": {
        "title": "Enabled",
        "description": "Whether the feature is enabled (true),
                        disabled (false), or under
                        automatic control (null)"
    }
}

It's just one example, but it is written in a way that title and description could be used independently without relying on the other for context.

[iscai-msft]

hey @dpwatrous, after discussing with my manager we've decided to fix this behavior for our v5 generators, but not to fix it in our v4 generator. Since Python mgmt and CLI have all migrated to our v5 generator, we've stopped support for it for a while. I'm really sorry for the pain this causes, the upside is when upgrading to v5 it will have the extra docstrings. Thank you so much again for all of your patience