Adding Swagger example files for all of ADLA and ADLS #1418

begoldsm · 2017-07-12T21:55:25Z

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.

Description

There are three pending issues with this change that were already discussed with the Swagger team. These errors are from the oav while calling "oav validate-example filesystem.json." Every Swagger file passed these tests except filesystem.json and its example files.

Because we were running into an "EQUIVALENT_PATH" error, we moved the paths into "x-ms-paths" and differentiated each path by inserting the "op" query and its associated constant string value into the path. We ran into the following form of error for all those paths:

error: Found errors in validating the request for x-ms-example "Creates a directory" in operation "FileSystem_Mkdirs".:

{ code: 'REQUEST_VALIDATION_ERROR',
  message: 'Found errors in validating the request for x-ms-example "Creates a directory" in operation "FileSystem_Mkdirs".',
  innerErrors:
   [ { code: 'INVALID_REQUEST_PARAMETER',
       errors:
        [ { code: 'REQUIRED',
            message: 'Value is required but was not provided',
            path:
             [ 'paths',
               '/webhdfs/v1/{path}?op=MKDIRS',
               'put',
               'parameters',
               '0' ] } ],
       in: 'path',
       message: 'Invalid parameter (path): Value is required but was not provided',
       name: 'path',
       path: { '$ref': '$["innerErrors"][0]["errors"][0]["path"]' } } ] }

There is currently no support for "application/octet-stream," which produced this error:
Issue: add support for application/octet-stream or file upload/download scenarios oav#136

error: Found errors in validating the request for x-ms-example "Appends to the specified file, optionally first creating the file if it does not yet ex
ist. This method supports multiple concurrent appends to the file. NOTE: The target must not contain data added by Create or normal (serial) Append. Co
ncurrentAppend and Append cannot be used interchangeably; once a target file has been modified using either of these append options, the other append o
ption cannot be used on the target file. ConcurrentAppend does not guarantee order and can result in duplicated data landing in the target file" in ope
ration "FileSystem_ConcurrentAppend".:

{ code: 'REQUEST_VALIDATION_ERROR',
  message: 'Found errors in validating the request for x-ms-example "Appends to the specified file, optionally first creating the file if it does not y
et exist. This method supports multiple concurrent appends to the file. NOTE: The target must not contain data added by Create or normal (serial) Appen
d. ConcurrentAppend and Append cannot be used interchangeably; once a target file has been modified using either of these append options, the other app
end option cannot be used on the target file. ConcurrentAppend does not guarantee order and can result in duplicated data landing in the target file" i
n operation "FileSystem_ConcurrentAppend".',
  innerErrors:
   [ { code: 'INVALID_CONTENT_TYPE',
       message: 'Invalid Content-Type (application/json).  These are supported: application/octet-stream',
       path: [] } ] }

Similarly to 2 above, there seems to be no support for returning "file" types in the response body:
Issue: add support for application/octet-stream or file upload/download scenarios oav#136

error: Found errors in validating the response with statusCode "200" for x-ms-example "Opens and reads from the specified file" in operation "FileSyste
m_Open".:

{ code: 'RESPONSE_VALIDATION_ERROR',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Opens and reads from the specified file" in operation "File
System_Open".',
  innerErrors:
   [ { code: 'INVALID_RESPONSE_BODY',
       errors:
        [ { code: undefined,
            message: 'Invalid \'type\' value: file',
            path: undefined } ],
       message: 'Invalid body: Invalid \'type\' value: file',
       path: [] } ] }

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.

… into current

This takes all of the new x-ms-examples from PR: * #9 And puts them into the correct format for the new branch structure.

Added new lines to the end of all example files to make GitHub happy. Fixed the ../examples to be ./examples due to the new folder structure.

azuresdkci · 2017-07-12T22:42:02Z

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/datalake-analytics/data-plane/readme.md
Before the PR: Warning(s): 0 Error(s): 0
After the PR: Warning(s): 0 Error(s): 0

File: specification/datalake-analytics/resource-manager/readme.md
Before the PR: Warning(s): 24 Error(s): 6
After the PR: Warning(s): 24 Error(s): 5

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

File: specification/datalake-store/resource-manager/readme.md
Before the PR: Warning(s): 7 Error(s): 6
After the PR: Warning(s): 7 Error(s): 5

Know more about AutoRest Linter Guidelines.

Send feedback and make AutoRest Linter Azure Bot smarter day by day!

Thanks for your co-operation.

begoldsm · 2017-07-13T17:18:23Z

@veronicagg @amarzavery could you guys take a look and help us understand why the x-ms-path validation examples are still failing?

… into current

Current PR is focused only on adding examples. I will make functional updates to the spec in a separate branch.

azuresdkci · 2017-07-13T17:35:41Z

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/datalake-analytics/data-plane/readme.md
Before the PR: Warning(s): 0 Error(s): 0
After the PR: Warning(s): 0 Error(s): 0

File: specification/datalake-analytics/resource-manager/readme.md
Before the PR: Warning(s): 22 Error(s): 6
After the PR: Warning(s): 22 Error(s): 5

File: specification/datalake-store/data-plane/readme.md
Before the PR: Warning(s): 9 Error(s): 8
After the PR: Warning(s): 1 Error(s): 27

File: specification/datalake-store/resource-manager/readme.md
Before the PR: Warning(s): 8 Error(s): 6
After the PR: Warning(s): 8 Error(s): 5

Know more about AutoRest Linter Guidelines.

Send feedback and make AutoRest Linter Azure Bot smarter day by day!

Thanks for your co-operation.

Fixed query params and moved one operation back into "paths" due to missing requirements documented in this github issue: Azure/autorest#2440

veronicagg · 2017-07-14T18:05:36Z

issue with x-ms-paths for example validation is tracked at Azure/oav#140

begoldsm · 2017-07-14T18:37:21Z

This PR is now approved for merge from the ADL team and confirmed to not cause code gen issues. Additionally, it resolves the following validation issues:
#802

veronicagg · 2017-07-14T19:13:41Z

taking over this PR, since I've been looking into the issues

veronicagg · 2017-07-14T19:31:07Z

@begoldsm Even though, diff shows changes in operations, it looks to be order changes, the PR appears to contain only addition of x-ms-examples. Please let me know if that's not the case.
Could you double check on the following:

Regarding linter errors, same linter errors should be coming up - so I assume you're aware of those:
Since some of the specs are data-plane and we're learning to categorize the rules, do you think the errors reported are applicable, like https://travis-ci.org/Azure/azure-rest-api-specs/jobs/253710677#L1633 ?
If you took a look at them and actually know that some of them are not applicable to data-plane, it'd be really helpful for us to know, so we could change the applicability of the rule.
Thanks!

veronicagg

PR generally looks good, just posted a few questions, before approving. thanks!

1. copy paste fail for LISTSTATUS 2. A change that should be in a different branch ended up in this PR. Reverted it (for the account_list payload).

begoldsm · 2017-07-14T20:54:54Z

@veronicagg I have an update:

https://travis-ci.org/Azure/azure-rest-api-specs/jobs/253710682#L1445
- This was a legitimate issue, which I just fixed in a new commit. Good catch!
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/253710682#L1770
- This was a legitimate issue, caused by an accidental commit in the wrong branch. I reverted the change to the example and this should now be fixed.
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/253710682#L1515
- This is a known issue with the validator: add support for application/octet-stream or file upload/download scenarios oav#136

As for the readme issue in data plane (https://travis-ci.org/Azure/azure-rest-api-specs/jobs/253710677#L1633):

LROs: There are no LROs in our data plane operations, so this is an invalid check for us.
Camel casing rules: Unfortunately we must adhere to the WebHDFS specification, which itself does not honor proper camel casing rules, so our hands are tied here (AclStatus cannot be "aclStatus" because it will break webHDFS compat). I think this validation error is specific to our swagger, though, so it is still valid for all others, we would just need an exception for the filesystem.json swagger file
Operations API requirement: This is specific to ARM, so should not be applied to data-plane swagger files (also we don't have an Operations API implemented for ARM, either, but that is a different problem 😄 ).
TrackedResourceListByImmediateParent: this needs to be a little less strict. The parent resource is DataLakeStoreAccount, but our method is called FirewallRules_ListByAccount for brevity. So it should notice that the list operation does still adhere to the "immediate parent" model.
EnumInsteadOfBoolean: This is showing up for the standard OData property $count. This is meant to be a boolean and has a very specific meaning in OData, so this should have an exception.

For the warnings I am not overly investigating since we have already discussed things like Guids and have your sign off 😄

AutorestCI · 2017-07-15T01:17:07Z

No modification for AutorestCI/azure-sdk-for-node

AutorestCI · 2017-07-15T01:17:29Z

No modification for AutorestCI/azure-sdk-for-ruby

AutorestCI · 2017-07-15T01:17:50Z

Azure/azure-sdk-for-python#1300

begoldsm added 3 commits July 12, 2017 14:38

Make readOnly properties readOnly

e839e47

Merge branch 'current' of https://github.com/Azure/azure-rest-api-specs…

6b58c26

… into current

Translation of master -> current for ADL

0edc6e7

This takes all of the new x-ms-examples from PR: * #9 And puts them into the correct format for the new branch structure.

msftclas added the cla-not-required label Jul 12, 2017

azuresdkci assigned jianghaolu Jul 12, 2017

Fix pathing and add newlines

3eb60d6

Added new lines to the end of all example files to make GitHub happy. Fixed the ../examples to be ./examples due to the new folder structure.

begoldsm added 2 commits July 13, 2017 10:24

Merge branch 'current' of https://github.com/Azure/azure-rest-api-specs…

cc1d856

… into current

Undoing readOnly change for this PR

56a719b

Current PR is focused only on adding examples. I will make functional updates to the spec in a separate branch.

Fixes to specs due to missing requirements

dafb5b1

Fixed query params and moved one operation back into "paths" due to missing requirements documented in this github issue: Azure/autorest#2440

Update examples to include OP now.

c8c9e0b

veronicagg assigned veronicagg and unassigned jianghaolu Jul 14, 2017

veronicagg suggested changes Jul 14, 2017

View reviewed changes

begoldsm added 2 commits July 14, 2017 13:41

Fix a couple validation issues

1ac234e

1. copy paste fail for LISTSTATUS 2. A change that should be in a different branch ended up in this PR. Reverted it (for the account_list payload).

Fix validation error because exception is required.

91d9730

Missed the x-ms-azure-resource tag

aef08c0

veronicagg approved these changes Jul 15, 2017

View reviewed changes

veronicagg merged commit d80feb5 into Azure:current Jul 15, 2017

AutorestCI mentioned this pull request Jul 15, 2017

Automatic PR from RestAPI-PR1418 Azure/azure-sdk-for-python#1300

Closed

begoldsm mentioned this pull request Jul 17, 2017

[DataLake Store - filesystem api-version: 2016-11-01] Semantic validation fails with duplicate API path errors #802

Closed

Adding Swagger example files for all of ADLA and ADLS #1418

Adding Swagger example files for all of ADLA and ADLS #1418

Uh oh!

Conversation

begoldsm commented Jul 12, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

PR information

Quality of Swagger

Uh oh!

azuresdkci commented Jul 12, 2017

Uh oh!

begoldsm commented Jul 13, 2017

Uh oh!

azuresdkci commented Jul 13, 2017

Uh oh!

veronicagg commented Jul 14, 2017

Uh oh!

begoldsm commented Jul 14, 2017

Uh oh!

veronicagg commented Jul 14, 2017

Uh oh!

veronicagg commented Jul 14, 2017

Uh oh!

veronicagg left a comment

Choose a reason for hiding this comment

Uh oh!

begoldsm commented Jul 14, 2017

Uh oh!

AutorestCI commented Jul 15, 2017

Uh oh!

AutorestCI commented Jul 15, 2017

Uh oh!

AutorestCI commented Jul 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

begoldsm commented Jul 12, 2017 •

edited by azuresdkci

Loading