Improve redis Swagger. #2969

TimLovellSmith · 2018-04-27T19:20:30Z

Documenting list all patchSchedules to fix #2968. Documenting the CheckNameAvailability.type property better to fix #2967. And using a realistic timespan value in PatchSchedule examples.

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

/cc @SiddharthChatrolaMs

AutorestCI · 2018-04-27T19:21:31Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-04-27T19:21:32Z

Automation for azure-libraries-for-java

The initial PR has been merged into your service PR:
AutorestCI/azure-libraries-for-java#122

AutorestCI · 2018-04-27T19:21:34Z

Automation for azure-sdk-for-node

Encountered a Subprocess error: (azure-sdk-for-node)

Command: ['/usr/local/bin/autorest', '/tmp/tmpnwblaga6/rest/specification/redis/resource-manager/readme.md', '--license-header=MICROSOFT_MIT_NO_VERSION', '--node-sdks-folder=/tmp/tmpnwblaga6/sdk', '--nodejs', '--use=@microsoft.azure/autorest.nodejs@^2.1.43']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4262; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@microsoft.azure_autorest-core@2.0.4272/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4272)
   Loading AutoRest extension '@microsoft.azure/autorest.nodejs' (^2.1.43->2.1.46)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
FATAL: System.InvalidOperationException: Sequence contains no matching element
   at System.Linq.Enumerable.First[TSource](IEnumerable1 source, Func2 predicate)
   at AutoRest.Extensions.SwaggerExtensions.RemoveUnreferencedTypes(CodeModel codeModel, HashSet`1 typeNames)
   at AutoRest.Extensions.SwaggerExtensions.FlattenModels(CodeModel codeModel)
   at AutoRest.Extensions.Azure.AzureExtensions.NormalizeAzureClientModel(CodeModel codeModel)
   at AutoRest.NodeJS.Azure.TransformerJsa.AutoRest.Core.ITransformer<AutoRest.NodeJS.Azure.Model.CodeModelJsa>.TransformCodeModel(CodeModel cm) in Z:\PUBLISHwybeg\46_20180403T154736\autorest.nodejs\src\azure\TransformerJsa.cs:line 34
   at AutoRest.NodeJS.Program.<ProcessInternal>d__3.MoveNext() in Z:\PUBLISHwybeg\46_20180403T154736\autorest.nodejs\src\Program.cs:line 125
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NewPlugin.<Process>d__15.MoveNext()
FATAL: nodejs/generate - FAILED
FATAL: Error: Plugin nodejs reported failure.
Process() cancelled due to exception : Plugin nodejs reported failure.
  Error: Plugin nodejs reported failure.

AutorestCI · 2018-04-27T19:21:41Z

Automation for azure-sdk-for-go

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-go#1778

azuresdkciprbot · 2018-04-27T19:25:09Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-27T19:25:10Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-27T19:30:51Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-27T19:30:51Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-27T19:36:48Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-27T19:36:48Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

TimLovellSmith · 2018-04-27T19:40:53Z

Autorest node generator appears to be crashing:

INFO:swaggertosdk.autorest_tools:   Loading AutoRest core      '/root/.autorest/@microsoft.azure_autorest-core@2.0.4275/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4275)

INFO:swaggertosdk.autorest_tools:   Installing AutoRest extension '@microsoft.azure/autorest.nodejs' (^2.1.43)

INFO:swaggertosdk.autorest_tools:   Installing AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44)

INFO:swaggertosdk.autorest_tools:FATAL: System.InvalidOperationException: Sequence contains no matching element

INFO:swaggertosdk.autorest_tools:   at System.Linq.Enumerable.First[TSource](IEnumerable1 source, Func2 predicate)

INFO:swaggertosdk.autorest_tools:   at AutoRest.Extensions.SwaggerExtensions.RemoveUnreferencedTypes(CodeModel codeModel, HashSet`1 typeNames)

INFO:swaggertosdk.autorest_tools:   at AutoRest.Extensions.SwaggerExtensions.FlattenModels(CodeModel codeModel)

INFO:swaggertosdk.autorest_tools:   at AutoRest.Extensions.Azure.AzureExtensions.NormalizeAzureClientModel(CodeModel codeModel)

INFO:swaggertosdk.autorest_tools:   at AutoRest.NodeJS.Azure.TransformerJsa.AutoRest.Core.ITransformer<AutoRest.NodeJS.Azure.Model.CodeModelJsa>.TransformCodeModel(CodeModel cm) in /opt/vsts/work/1/s/src/azure/TransformerJsa.cs:line 34

INFO:swaggertosdk.autorest_tools:   at AutoRest.NodeJS.Program.<ProcessInternal>d__3.MoveNext() in /opt/vsts/work/1/s/src/Program.cs:line 125

INFO:swaggertosdk.autorest_tools:--- End of stack trace from previous location where exception was thrown ---

INFO:swaggertosdk.autorest_tools:   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

INFO:swaggertosdk.autorest_tools:   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

INFO:swaggertosdk.autorest_tools:   at NewPlugin.<Process>d__15.MoveNext()

INFO:swaggertosdk.autorest_tools:FATAL: nodejs/generate - FAILED

INFO:swaggertosdk.autorest_tools:FATAL: Error: Plugin nodejs reported failure.

INFO:swaggertosdk.autorest_tools:Process() cancelled due to exception : Plugin nodejs reported failure.

INFO:swaggertosdk.autorest_tools:  Error: Plugin nodejs reported failure.

jianghaolu · 2018-04-27T23:22:28Z

specification/redis/resource-manager/Microsoft.Cache/stable/2018-03-01/redis.json

      "properties": {
        "name": {
          "type": "string",
          "description": "Resource name."
        },
        "type": {
          "type": "string",
-          "description": "Resource type."
+          "description": "Resource type to check name availability of, e.g. 'Microsoft.Cache/redis'.",
+          "enum": [


Is it okay to keep this as string? Changing the type from string to enum (even with modelAsString) will be a breaking change in many languages.

@jhendrixMSFT What say you?

I'm torn on this as breaking changes stink however in this case there was no clue what to pass for the value (I had to look at the checked in example JSON to figure it out). If we really don't want to take the breaking change then the doc string will have to be the source of truth.
Another thing to consider, if this param will only ever have the value "Microsoft.Cache/redis" then if you mark it "modelAsString": false AutoRest should treat it as a constant and remove the param entirely (i.e. hard-coding the value in the request).

@jhendrixMSFT
For this api-version we don't have any other resource types, and if we ever have a new resource we can use a new api-version for it. So I think it's fine to treat it as a constant.
Let's try that out and see if that makes everyone happy!
/cc @jianghaolu

azuresdkciprbot · 2018-04-30T21:49:24Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-30T21:49:24Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

TimLovellSmith · 2018-04-30T21:56:46Z

I see linter validation errors..
I think it is just because the parameter got removed and I need to remove it from the example too.
Trying that now..

`
error: Found errors in validating the request for x-ms-example "RedisCacheList" in operation "Redis_CheckNameAvailability".:

{ code: 'REQUEST_VALIDATION_ERROR',

id: 'OAV109',

message: 'Found errors in validating the request for x-ms-example "RedisCacheList" in operation "Redis_CheckNameAvailability".',

innerErrors:

[ { code: 'INVALID_REQUEST_PARAMETER',

   errors: 

    [ { code: 'ONE_OF_MISSING',

        params: [],

        message: 'Data does not match any schemas from \'oneOf\'',

        path: [ 'type' ],

        inner: 

         [ { code: 'ENUM_MISMATCH',

             params: [ 'Microsoft.Cache/Redis' ],

             message: 'No enum match for: Microsoft.Cache/Redis',

             path: [ 'type' ],

             description: 'Resource type to check name availability of, e.g. \'Microsoft.Cache/redis\'.' },

           { code: 'INVALID_TYPE',

             params: [ 'null', 'string' ],

             message: 'Expected type null but found type string',

             path: [ 'type' ] } ] } ],

   in: 'body',

   message: 'Invalid parameter (parameters): Value failed JSON Schema validation',

   name: 'parameters',

   path: 

    [ 'paths',

      '/subscriptions/{subscriptionId}/providers/Microsoft.Cache/CheckNameAvailability',

      'post',

      'parameters',

      '0' ] } ] }

{ AssertionError [ERR_ASSERTION]: swagger "specification/redis/resource-manager/Microsoft.Cache/stable/2018-03-01/redis.json" contains model validation errors.

at Context.<anonymous> (/home/travis/build/Azure/azure-rest-api-specs/test/model.js:20:16)

at <anonymous>

generatedMessage: false,

name: 'AssertionError [ERR_ASSERTION]',

code: 'ERR_ASSERTION',

actual: false,

expected: true,

operator: '==' }

1 passing (3s)

1 failing

Azure swagger model validation using x-ms-examples and examples in spec

specification/redis/resource-manager/Microsoft.Cache/stable/2018-03-01/redis.json should have valid examples.:

AssertionError [ERR_ASSERTION]: swagger "specification/redis/resource-manager/Microsoft.Cache/stable/2018-03-01/redis.json" contains model validation errors.
- expected - actual
-false

+true

at Context. (test/model.js:20:16)

at
`

azuresdkciprbot · 2018-04-30T22:37:31Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-30T22:37:31Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

TimLovellSmith · 2018-04-30T22:47:44Z

@jhendrixMSFT @jianghaolu
Looks like removing the parameter and adding the enum type still gets flagged as a breaking change by AutoRest. Do you think that this is really a good idea?

If it were possible, I would rather do this in a non-breaking way (i.e. keep it string) but with a 'default' parameter value supplied, so that anyone writing new code wouldn't have to feel confused about what type to supply. Do you know if that is possible?

jhendrixMSFT · 2018-04-30T22:50:28Z

The question of default value has come up before and I don't believe there is support for this. So if we want to do it in a non-breaking way the only real option that I know of is to update the comment like you already did.
CC @fearthecowboy

TimLovellSmith · 2018-05-01T20:14:26Z

@jhendrixMSFT Given that we have other issues with CheckNameAvailability, which might be another breaking change, and a new api-version to fix (I refer to #2981) I think the best thing to do is NOT do any breaking change to the generated code for this api-version, but just document it for this api version and CONSIDER modeling it as an enum next api-version, and then people will hopefully notice the return type is changing at the same time as they are fixing up build issues for the number of arguments change.

So, for this PR, I am just going to improve the documentation, and revert the model as enum changes. I hope this makes sense.

/cc @fearthecowboy @hovsepm @jianghaolu

…zure#2968. Documenting the 'CheckNameAvailability.type' property better to address Azure#2967 (and marking properties as required). Also using a more realistic timespan value in the PatchSchedule examples.

…3-01).

azuresdkciprbot · 2018-05-01T20:29:48Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-05-01T20:29:48Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/redis/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

TimLovellSmith · 2018-05-02T22:13:33Z

@jhendrixMSFT @jianghaolu Is this PR good enough now?

TimLovellSmith · 2018-05-05T01:01:30Z

@jhendrixMSFT @jianghaolu Thanks for the review!

azuresdkci assigned jianghaolu Apr 27, 2018

azuresdkci requested a review from jianghaolu April 27, 2018 19:21

TimLovellSmith force-pushed the redisApril2018Fixes branch from c23f54a to d7abeaa Compare April 27, 2018 19:26

This was referenced Apr 27, 2018

[AutoPR redis/resource-manager] Improve redis Swagger. Azure/azure-sdk-for-python#2485

Merged

[AutoPR redis/resource-manager] Improve redis Swagger. AutorestCI/azure-libraries-for-java#121

Merged

AutorestCI mentioned this pull request Apr 27, 2018

[AutoPR redis/resource-manager] Improve redis Swagger. Azure/azure-sdk-for-go#1719

Closed

jianghaolu reviewed Apr 27, 2018

View reviewed changes

TimLovellSmith force-pushed the redisApril2018Fixes branch from 4f7e77b to cdc4da5 Compare April 30, 2018 22:23

TimLovellSmith added 2 commits May 1, 2018 13:20

Improve redis Swagger. Documenting 'list all patchSchedules' to fix A…

34aa2a2

…zure#2968. Documenting the 'CheckNameAvailability.type' property better to address Azure#2967 (and marking properties as required). Also using a more realistic timespan value in the PatchSchedule examples.

Backport to the Microsoft.cache/redis 2017-10-01 swagger (from 2018-0…

44e3bae

…3-01).

TimLovellSmith force-pushed the redisApril2018Fixes branch from cdc4da5 to 44e3bae Compare May 1, 2018 20:25

AutorestCI mentioned this pull request May 1, 2018

[AutoPR redis/resource-manager] Improve redis Swagger. Azure/azure-sdk-for-go#1748

Closed

jhendrixMSFT approved these changes May 2, 2018

View reviewed changes

jianghaolu merged commit 6b8cb30 into Azure:master May 3, 2018

AutorestCI mentioned this pull request May 3, 2018

[AutoPR redis/resource-manager] Improve redis Swagger. Azure/azure-sdk-for-go#1777

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve redis Swagger. #2969

Improve redis Swagger. #2969

TimLovellSmith commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

azuresdkciprbot commented Apr 27, 2018

azuresdkciprbot commented Apr 27, 2018

azuresdkciprbot commented Apr 27, 2018

azuresdkciprbot commented Apr 27, 2018

azuresdkciprbot commented Apr 27, 2018

azuresdkciprbot commented Apr 27, 2018

TimLovellSmith commented Apr 27, 2018

jianghaolu Apr 27, 2018

TimLovellSmith Apr 27, 2018

jhendrixMSFT Apr 30, 2018

TimLovellSmith Apr 30, 2018

azuresdkciprbot commented Apr 30, 2018

azuresdkciprbot commented Apr 30, 2018

TimLovellSmith commented Apr 30, 2018

azuresdkciprbot commented Apr 30, 2018

azuresdkciprbot commented Apr 30, 2018

TimLovellSmith commented Apr 30, 2018

jhendrixMSFT commented Apr 30, 2018

TimLovellSmith commented May 1, 2018

azuresdkciprbot commented May 1, 2018

azuresdkciprbot commented May 1, 2018

TimLovellSmith commented May 2, 2018

TimLovellSmith commented May 5, 2018

Improve redis Swagger. #2969

Improve redis Swagger. #2969

Conversation

TimLovellSmith commented Apr 27, 2018 • edited Loading

PR information

Quality of Swagger

AutorestCI commented Apr 27, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Apr 27, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented Apr 27, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Apr 27, 2018 • edited Loading

Automation for azure-sdk-for-go

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

TimLovellSmith commented Apr 27, 2018

jianghaolu Apr 27, 2018

Choose a reason for hiding this comment

TimLovellSmith Apr 27, 2018

Choose a reason for hiding this comment

jhendrixMSFT Apr 30, 2018

Choose a reason for hiding this comment

TimLovellSmith Apr 30, 2018

Choose a reason for hiding this comment

azuresdkciprbot commented Apr 30, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented Apr 30, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

TimLovellSmith commented Apr 30, 2018

azuresdkciprbot commented Apr 30, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented Apr 30, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

TimLovellSmith commented Apr 30, 2018

jhendrixMSFT commented Apr 30, 2018

TimLovellSmith commented May 1, 2018

azuresdkciprbot commented May 1, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

azuresdkciprbot commented May 1, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

TimLovellSmith commented May 2, 2018

TimLovellSmith commented May 5, 2018

TimLovellSmith commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading

AutorestCI commented Apr 27, 2018 •

edited

Loading