Service Fabric REST API specification version 6.2 RTO by vipul-modi · Pull Request #2855 · Azure/azure-rest-api-specs

vipul-modi · 2018-04-12T00:19:16Z

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. (Not Applicable)
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

[X ] I have read the contribution guidelines.
[ X] 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.

AutorestCI · 2018-04-12T00:22:25Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-04-12T00:23:43Z

Automation for azure-libraries-for-java

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

AutorestCI · 2018-04-12T00:27:49Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-04-12T00:30:30Z

Automation for azure-sdk-for-go

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

annatisch · 2018-04-12T14:19:49Z

...efabric/data-plane/Microsoft.ServiceFabric/stable/6.2/examples/GetServiceTypeInfoList-1.json

+              {
+                "Key": "__GeneratedServiceType__",
+                "Value": null,
+                "\"<GeneratedNames xmlns=\"http": //schemas.microsoft.com/2015/03/fabact-no-schema"> <DefaultService Name="WebServerService" /> <ServiceEndpoint Name="WebServerTypeEndpoint" /> </GeneratedNames>"


Some invalid JSON here

annatisch · 2018-04-12T14:19:57Z

...abric/data-plane/Microsoft.ServiceFabric/stable/6.2/examples/GetServiceTypeInfoByName-1.json

+            {
+              "Key": "__GeneratedServiceType__",
+              "Value": null,
+              "\"<GeneratedNames xmlns=\"http": //schemas.microsoft.com/2015/03/fabact-no-schema"> <DefaultService Name="WebServerService" /> <ServiceEndpoint Name="WebServerTypeEndpoint" /> </GeneratedNames>"


Some invalid JSON here

thanks let me look in to it.

vipul-modi · 2018-04-13T16:05:14Z

@annatisch - the issue should be resolved now. This should have been caught with our internal CI/build process on the Service Fabric swagger specification repo. We will improve this. @salameer

annatisch · 2018-04-13T16:27:56Z

Thanks @VipulM-MSFT.
There are a number of errors in both the model and linter PR_ONLY=true CI jobs, for example there are many properties with this:

"Property named: 'PackageSharingPolicy', for definition: 'DeployServicePackageToNodeDescription' must follow camelCase style. Example: 'packageSharingPolicy'.",

And many of the models:

Found errors in validating the response with statusCode "200" for x-ms-example "Get Correlated events" in operation "GetCorrelatedEventList".:

Could you take a look?

vipul-modi · 2018-04-13T16:52:25Z

@annatisch - this is not an Azure API specifications so the naming suppressions are already part of the README.MD and approved @salameer .

@salameer - do you know why CI is not picking up the suppressions from the README.MD? This job https://travis-ci.org/Azure/azure-rest-api-specs/jobs/365940147 has failures that are already suppressed in README.

vipul-modi · 2018-04-13T16:57:31Z

We also suppress the example validations due to issues in the validation tooling. Once they are fixed we can re-enable the checks and those failures should turn in to blocking failures. @salameer has the context on the issues.

samedder · 2018-04-13T23:33:08Z

@VipulM-MSFT I believe the supressions need to be in the README.md at the root of all the versions, the same one that contains the tags for generation. Take a look at this pr that added supressions.

vipul-modi · 2018-04-13T23:59:01Z

#hold-off

There are genuine failures in the example validation, we will fix and update the PR with those and ensure that the suppressions are added at the root. Let's wait on merging this PR for now, until I and @samedder sign off again.

…mple validation

…nder data-plane directory.

azuresdkciprbot · 2018-04-19T00:06:16Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/servicefabric/data-plane/readme.md
Before the PR: Warning(s): 9 Error(s): 715
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

vipul-modi · 2018-04-19T00:47:35Z

@salameer and @annatisch this is ready to go. All issues should be fixed now. #sign-off

vipul-modi · 2018-04-20T04:03:12Z

Ping - @salameer and @annatisch

annatisch · 2018-04-20T19:05:22Z

@VipulM-MSFT
I can't leave comments on the diff itself because it's too large for Github to load it.
Can you please remove the newline characters from the end of descriptions, for example:

    "BackupSuspensionInfo": {
      "type": "object",
      "description": "Describes the backup suspension details.\n",
      "properties": {
        "IsSuspended": {
          "type": "boolean",
          "description": "Indicates whether periodic backup is suspended at this level or not."
        },
        "SuspensionInheritedFrom": {
          "$ref": "#/definitions/BackupSuspensionScope",
          "description": "Specifies the scope at which the backup suspension was applied.\n"
        }
      }
    },

This creates strange formatting in the generated SDKs for example:

class BackupSuspensionInfo(Model):
    """Describes the backup suspension details.
    .

vipul-modi · 2018-04-20T19:36:59Z

@annatisch - the SDK generators should trim before generating the descriptions. I have removed the newlines. If there are no other issues, please publish the specification.

azuresdkciprbot · 2018-04-20T19:40:23Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/servicefabric/data-plane/readme.md
Before the PR: Warning(s): 9 Error(s): 715
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

Wandisco.Fusion - Creating new API version 2020-12-01-preview

Service Fabric REST API specification version 6.2 RTO

4cffa20

azuresdkci assigned annatisch Apr 12, 2018

azuresdkci requested a review from annatisch April 12, 2018 00:19

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR servicefabric/data-plane] Service Fabric REST API specification version 6.2 RTO Azure/azure-sdk-for-python#2383

Merged

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR servicefabric/data-plane] Service Fabric REST API specification version 6.2 RTO AutorestCI/azure-libraries-for-java#70

Merged

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR servicefabric/data-plane] Service Fabric REST API specification version 6.2 RTO Azure/azure-sdk-for-node#2720

Merged

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR servicefabric/data-plane] Service Fabric REST API specification version 6.2 RTO Azure/azure-sdk-for-go#1563

Closed

annatisch reviewed Apr 12, 2018

View reviewed changes

remove unused example and fix the ServiceTypeInfo examples

0441537

samedder approved these changes Apr 13, 2018

View reviewed changes

fix the bugs in the specification and examples as surfaced by the exa…

9ceed58

…mple validation

AutorestCI mentioned this pull request Apr 18, 2018

[AutoPR servicefabric/data-plane] Service Fabric REST API specification version 6.2 RTO Azure/azure-sdk-for-go#1633

Merged

vipul-modi added 6 commits April 17, 2018 18:37

add suppressions at the higherlevel readme.md file.

c3759cd

use new format for suppressions

1bcb57e

remove readme from version specific folder and keep a master readme u…

ca4a0ab

…nder data-plane directory.

triggering the Travis CI build again.

ad90a8c

example updates

dff9557

remove dupplicate suppressions

65c04ac

remove newlines from the end of the description

813b945

annatisch approved these changes Apr 20, 2018

View reviewed changes

annatisch merged commit f037370 into Azure:master Apr 20, 2018

mccleanp pushed a commit that referenced this pull request Mar 23, 2022

Merge pull request #2855 from dafunker/wd_ghamilt_Azure_11196

06f59d5

Wandisco.Fusion - Creating new API version 2020-12-01-preview

Conversation

vipul-modi commented Apr 12, 2018 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

AutorestCI commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

AutorestCI commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-libraries-for-java

Uh oh!

AutorestCI commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-node

Uh oh!

AutorestCI commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

annatisch Apr 12, 2018

Choose a reason for hiding this comment

Uh oh!

annatisch Apr 12, 2018

Choose a reason for hiding this comment

Uh oh!

vipul-modi Apr 13, 2018

Choose a reason for hiding this comment

Uh oh!

vipul-modi commented Apr 13, 2018

Uh oh!

annatisch commented Apr 13, 2018

Uh oh!

vipul-modi commented Apr 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vipul-modi commented Apr 13, 2018

Uh oh!

samedder commented Apr 13, 2018

Uh oh!

vipul-modi commented Apr 13, 2018

Uh oh!

azuresdkciprbot commented Apr 19, 2018

Uh oh!

vipul-modi commented Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vipul-modi commented Apr 20, 2018

Uh oh!

annatisch commented Apr 20, 2018

Uh oh!

vipul-modi commented Apr 20, 2018

Uh oh!

azuresdkciprbot commented Apr 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

vipul-modi commented Apr 12, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

vipul-modi commented Apr 13, 2018 •

edited

Loading

vipul-modi commented Apr 19, 2018 •

edited

Loading