[ADLA - ADLS] - Adding pageable extension for Catalog ACL APIs and removing childrenNum property by ro-joowan · Pull Request #2353 · Azure/azure-rest-api-specs

ro-joowan · 2018-01-29T03:20:14Z

Adding pageable extension for Catalog ACL APIs
Removing childrenNum property from FileStatusProperties as noted in this issue: Azure Data Lake Store SDK : FileStatusProperties.ChildrenNum Property always null azure-sdk-for-net#3770

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.

* Although paging is not supported for these operations, the design decision that we took was to provide support at the client side just in case we need to add it from the server side.

* This property is always null as brought up by this issue: Azure/azure-sdk-for-net#3770

AutorestCI · 2018-01-29T03:22:45Z

Did a commit to Azure/azure-sdk-for-python:
Azure/azure-sdk-for-python@be62c19

AutorestCI · 2018-01-29T03:26:21Z

Did a commit to Azure/azure-sdk-for-go:
Azure/azure-sdk-for-go@b5e3dc0

AutorestCI · 2018-01-29T03:49:39Z

This commit was treated and no generation was made for Azure/azure-sdk-for-python

azuresdkciprbot · 2018-01-29T03:51:17Z

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

AutorestCI · 2018-01-29T03:53:12Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

anuchandy · 2018-01-30T00:56:12Z

Please take a look and fix the example validation errors

anuchandy · 2018-01-29T21:41:30Z

...ion/datalake-analytics/data-plane/Microsoft.DataLakeAnalytics/stable/2016-11-01/catalog.json

  },
  "x-ms-paths": {
-    "/catalog/usql/databases/{databaseName}/acl?op=GRANTACE": {
+    "/catalog/usql/acl?op=GRANTACE": {


[For the record] Looking at the diff, at first glance it may appear that there are breaking changes in the following endpoints defs but not really - they appear so due to re-ordering of the operations in the swagger.

Catalog_GrantAclToDatabase Catalog_GrantAcl Catalog_RevokeAclFromDatabase Catalog_GrantAcl Catalog_RevokeAcl Catalog_RevokeAclFromDatabase

@ro-joowan if possible, for future PRs it will be very helpful if we can ensure that PR shows only the changed sections.

@anuchandy, Noted :) I should've put that part in a separate commit. Sorry about that!

anuchandy · 2018-01-30T00:36:24Z

...ion/datalake-analytics/data-plane/Microsoft.DataLakeAnalytics/stable/2016-11-01/catalog.json

-          },
-          "default": {
-            "description": "404 - Not found."
+            "description": "Successfully granted the access control list (ACL) entry to the Data Lake Analytics catalog."


Why we are removing the default from responses section?

@anuchandy This was to follow the the rest of the APIs in that file. These ACL APIs were the first ones that I added to this Swagger file. But now as I look at them, I see that default is not really needed (or is redundant) because if it's not 200, we throw with the status code.

anuchandy · 2018-01-30T00:59:50Z

We are removing childrenNum property but bit confused why we are removing this? Is it the case that service doesn't support this and always return null?

ro-joowan · 2018-01-30T01:11:05Z

@anuchandy Yes, the service doesn't support that property and it is always null.

ro-joowan · 2018-01-30T02:20:32Z

@anuchandy I have addressed the example errors in my latest commit. Thank you

azuresdkciprbot · 2018-01-30T02:29:36Z

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

AutorestCI · 2018-01-30T02:33:43Z

This commit was treated and no generation was made for Azure/azure-sdk-for-python

AutorestCI · 2018-01-30T02:42:37Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

anuchandy · 2018-01-30T18:54:03Z

@ro-joowan thanks for the response and fixing the model errors.

Summary:

Removed the property childrenNum because presence of this property was bug in swagger. Service never return such a property so client always see null for this. Related SOF issue
Changed AclList to pageable.
Removed bunch of default entries from the response section of Catalog ACL operations in order to consistent with other operation groups. These default has not model defined hence they were only used for doc purposes.

AutorestCI · 2018-01-30T19:02:06Z

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

ro-joowan added 3 commits January 28, 2018 18:42

Adding paging for ListAcls and ListAclsByDatabase

969e5d6

* Although paging is not supported for these operations, the design decision that we took was to provide support at the client side just in case we need to add it from the server side.

Removing default response from Swagger for Acl APIs

5cf7c4c

Removing childrenNum property from FileStatusProperties

7579d9c

* This property is always null as brought up by this issue: Azure/azure-sdk-for-net#3770

AutorestCI mentioned this pull request Jan 29, 2018

Automatic PR of restapi_auto_ro-joowan_catalog-pageable-store-bugfix-childproperty into master Azure/azure-sdk-for-python#1843

Closed

AutorestCI mentioned this pull request Jan 29, 2018

Automatic PR of restapi_auto_ro-joowan_catalog-pageable-store-bugfix-childproperty into dev Azure/azure-sdk-for-go#1004

Closed

azuresdkci assigned anuchandy Jan 29, 2018

Fixing examples for childrenNum

94cbecb

anuchandy requested changes Jan 30, 2018

View reviewed changes

Fixing oav validate-example errors

d97b895

anuchandy added the potential-sdk-breaking-change label Jan 30, 2018

anuchandy approved these changes Jan 30, 2018

View reviewed changes

anuchandy merged commit d3af35e into Azure:master Jan 30, 2018

ro-joowan deleted the catalog-pageable-store-bugfix-childproperty branch January 30, 2018 19:05

This was referenced Feb 5, 2018

[ADLA] Account APIs object refactor and adding paging extension to Catalog ACL APIs Azure/azure-sdk-for-net#4043

Merged

[ADLS] Account APIs object refactor and couple of bugfixes Azure/azure-sdk-for-net#4046

Merged

idear1203 mentioned this pull request May 18, 2018

[ADLA] - Adding PS support for Catalog ACLs (v1) Azure/azure-powershell#6252

Merged

8 tasks

Conversation

ro-joowan commented Jan 29, 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 Jan 29, 2018

Uh oh!

AutorestCI commented Jan 29, 2018

Uh oh!

AutorestCI commented Jan 29, 2018

Uh oh!

azuresdkciprbot commented Jan 29, 2018

Uh oh!

AutorestCI commented Jan 29, 2018

Uh oh!

anuchandy commented Jan 30, 2018

Uh oh!

anuchandy Jan 29, 2018

Choose a reason for hiding this comment

Uh oh!

ro-joowan Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

anuchandy Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

ro-joowan Jan 30, 2018

Choose a reason for hiding this comment

Uh oh!

anuchandy commented Jan 30, 2018

Uh oh!

ro-joowan commented Jan 30, 2018

Uh oh!

ro-joowan commented Jan 30, 2018

Uh oh!

azuresdkciprbot commented Jan 30, 2018

Uh oh!

AutorestCI commented Jan 30, 2018

Uh oh!

AutorestCI commented Jan 30, 2018

Uh oh!

anuchandy commented Jan 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AutorestCI commented Jan 30, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ro-joowan commented Jan 29, 2018 •

edited by azuresdkci

Loading

anuchandy commented Jan 30, 2018 •

edited

Loading