[ADLA] - Adding PS support for Catalog ACLs (v1) by idear1203 · Pull Request #6252 · Azure/azure-powershell

idear1203 · 2018-05-18T08:04:44Z

Description

Swagger PR Links

Changes

Update Microsoft.Azure.Management.DataLake.Analytics package to version 3.3.0-preview and fix corresponding compilation errors
Adding initial set of cmdlets to support catalog ACLs.
- Cmdlets design review: azure-powershell-cmdlet-review-pr#72

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

MiYanni · 2018-06-06T01:23:29Z

@idear1203 You have merge conflicts that need to be resolved.

idear1203 · 2018-06-08T06:41:18Z

@MiYanni Thanks for informing the issue. I have fixed the merge conflicts. Please go ahead review the PR.

maddieclayton · 2018-06-11T18:43:50Z

@idear1203 To fix the current Jenkins failure, please update the profile changelog (to ensure your common code changes are picked up in the next release and documented): https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Profile/ChangeLog.md

idear1203 · 2018-06-21T03:37:44Z

@cormacpayne Could you please go ahead to review this PR? Note I have put -User and -Group options backs because I find that I cannot access AD calls such as Get-AzureRmADUser via service principal even if I grant permissions to the service principal.

cormacpayne

@idear1203 some comments for you to take a look at

cormacpayne · 2018-06-22T15:30:16Z

...lytics/Commands.DataLakeAnalytics/Commands/GetAzureRmDataLakeAnalyticsCatalogItemAclEntry.cs

@idear1203 nit: you can remove the List<PSDataLakeAnalyticsAcl> output type from this line and just have PSDataLakeAnalyticsAcl #Resolved

cormacpayne · 2018-06-22T15:33:13Z

...lytics/Commands.DataLakeAnalytics/Commands/GetAzureRmDataLakeAnalyticsCatalogItemAclEntry.cs

@idear1203 you can remove the ValueFromPipelineByPropertyName and Position properties from parameters that are of type SwitchParameter (this comment applies to all files) #Resolved

cormacpayne · 2018-06-22T15:34:08Z

...lytics/Commands.DataLakeAnalytics/Commands/GetAzureRmDataLakeAnalyticsCatalogItemAclEntry.cs

@idear1203 since this is a list, make sure that each object in the list is individually written to the output stream by changing this to WriteObject(toReturn, true) #Resolved

cormacpayne · 2018-06-22T15:36:34Z

...ics/Commands.DataLakeAnalytics/Commands/RemoveAzureRmDataLakeAnalyticsCatalogItemAclEntry.cs

@idear1203 is the -Force parameter necessary? We suggest using the -Force parameter when the cmdlet performs additional prompting that the user should be able to skip (e.g., deleting an empty folder requires no prompting, but deleting a folder with files in it will prompt the user asking if they want to delete the files too). If this cmdlet doesn't do any additional prompting, then you should remove the -Force parameter. #Resolved

Thanks! -Force will be removed #Resolved

cormacpayne · 2018-06-22T15:37:25Z

...lytics/Commands.DataLakeAnalytics/Commands/SetAzureRmDataLakeAnalyticsCatalogItemAclEntry.cs

@idear1203 nit: same comment here about removing List<PSDataLakeAnalyticsAcl> #Resolved

cormacpayne · 2018-06-22T15:40:43Z

...lytics/Commands.DataLakeAnalytics/Commands/SetAzureRmDataLakeAnalyticsCatalogItemAclEntry.cs

@idear1203 same comment about changing this line to WriteObject(toReturn, true) #Resolved

cormacpayne · 2018-06-22T15:42:32Z

...sourceManager/DataLakeAnalytics/Commands.DataLakeAnalytics/Models/DataLakeAnalyticsClient.cs

@idear1203 is this being used anywhere? #Resolved

No. I will remove this line #Resolved

cormacpayne · 2018-06-22T15:43:50Z

...Analytics/Commands.DataLakeAnalytics/help/Get-AzureRmDataLakeAnalyticsCatalogItemAclEntry.md

@idear1203 for the examples you're adding in this PR, would you be able to add the corresponding output from the cmdlet call so users have an idea of what is returned? #Resolved

Output are added for examples of Get-AzureRmDataLakeAnalyticsCatalogItemAclEntry and Set-AzureRmDataLakeAnalyticsCatalogItemAclEntry

In reply to: 197487475 [](ancestors = 197487475)

cormacpayne · 2018-06-22T15:44:48Z

...Analytics/Commands.DataLakeAnalytics/help/Get-AzureRmDataLakeAnalyticsCatalogItemAclEntry.md

@idear1203 this line can be changed to ### Microsoft.Azure.Commands.DataLakeAnalytics.Models.PSDataLakeAnalyticsAcl #Resolved

cormacpayne · 2018-06-22T15:45:46Z

...Analytics/Commands.DataLakeAnalytics/help/Set-AzureRmDataLakeAnalyticsCatalogItemAclEntry.md

@idear1203 same comment about changing this line to ### Microsoft.Azure.Commands.DataLakeAnalytics.Models.PSDataLakeAnalyticsAcl #Resolved

idear1203 · 2018-06-22T17:30:22Z

@cormacpayne Thanks for your comments! I have resolved all of them. Please take a look.

This change aims to update Microsoft.Azure.Management.DataLake.Analytics to version 3.3.0-preview, and fix the breaking changes due to the version upgrade

* Add support for Catalog ACLs through the following commands: - Get-AzureRmDataLakeAnalyticsCatalogItemAclEntry - Set-AzureRmDataLakeAnalyticsCatalogItemAclEntry - Remove-AzureRmDataLakeAnalyticsCatalogItemAclEntry

idear1203 · 2018-06-23T03:06:44Z

@vladimir-shcherbakov Merge conflicts have been resolved

cormacpayne

@idear1203 a few minor comments, otherwise LGTM

cormacpayne · 2018-06-25T16:39:56Z

...ger/DataLakeAnalytics/Commands.DataLakeAnalytics.Test/Commands.DataLakeAnalytics.Test.csproj

@idear1203 nit: we can go ahead and remove this project reference since we aren't using the RBAC client #Resolved

cormacpayne · 2018-06-25T16:40:37Z

...urceManager/DataLakeAnalytics/Commands.DataLakeAnalytics.Test/ScenarioTests/AdlaTestsBase.cs

@idear1203 nit: we should remove this client since we don't use it anywhere in the cmdlet code #Resolved

cormacpayne · 2018-06-25T16:41:15Z

...eManager/DataLakeAnalytics/Commands.DataLakeAnalytics/AzureRM.DataLakeAnalytics.Netcore.psd1

@idear1203 it looks like this line was repeated #Resolved

idear1203 · 2018-06-26T02:28:37Z

@cormacpayne Thanks, I've addressed your comment. Please take a look.

cormacpayne · 2018-06-26T16:01:14Z

On-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/963/

azuresdkci added the needs-review label May 18, 2018

idear1203 force-pushed the add_ps_support_for_catalog_acls branch 4 times, most recently from b2dfc90 to b34016c Compare May 21, 2018 14:32

maddieclayton added the Cmdlet Review Required ⚠️ label May 21, 2018

idear1203 force-pushed the add_ps_support_for_catalog_acls branch from b34016c to 30e418f Compare May 29, 2018 07:59

maddieclayton removed the needs-review label Jun 1, 2018

idear1203 force-pushed the add_ps_support_for_catalog_acls branch from 30e418f to 7e123df Compare June 6, 2018 15:00

maddieclayton added the Merge Conflicts label Jun 6, 2018

cormacpayne assigned idear1203 Jun 6, 2018

idear1203 force-pushed the add_ps_support_for_catalog_acls branch 2 times, most recently from 2024d96 to d244b3c Compare June 8, 2018 06:28

idear1203 assigned markcowl and unassigned idear1203 Jun 11, 2018

idear1203 force-pushed the add_ps_support_for_catalog_acls branch from d244b3c to d561b17 Compare June 11, 2018 03:37

cormacpayne assigned idear1203 and unassigned markcowl Jun 11, 2018

cormacpayne added needs-revision and removed Merge Conflicts labels Jun 11, 2018

idear1203 force-pushed the add_ps_support_for_catalog_acls branch 3 times, most recently from f56354c to 792fe95 Compare June 11, 2018 08:47

maddieclayton added Cmdlet Review Complete and removed Cmdlet Review Required ⚠️ labels Jun 11, 2018

maddieclayton assigned praries880 Jun 11, 2018

maddieclayton added the Do Not Merge 🚫 label Jun 11, 2018

cormacpayne suggested changes Jun 22, 2018

View reviewed changes

idear1203 force-pushed the add_ps_support_for_catalog_acls branch from 0363fdf to c95d0e4 Compare June 22, 2018 17:28

vladimir-shcherbakov assigned cormacpayne and unassigned idear1203 Jun 22, 2018

vladimir-shcherbakov added the Merge Conflicts label Jun 22, 2018

Dongwei Wang added 3 commits June 23, 2018 10:57

[ADLA] - Update DataLakeAnalytics .NET SDK version to 3.3.0-preview

a3be85c

This change aims to update Microsoft.Azure.Management.DataLake.Analytics to version 3.3.0-preview, and fix the breaking changes due to the version upgrade

[ADLA] - Adding PS support for Catalog ACLs (v1)

bac18cf

* Add support for Catalog ACLs through the following commands: - Get-AzureRmDataLakeAnalyticsCatalogItemAclEntry - Set-AzureRmDataLakeAnalyticsCatalogItemAclEntry - Remove-AzureRmDataLakeAnalyticsCatalogItemAclEntry

[ADLA] - Add -User and -Group option

e43e247

idear1203 force-pushed the add_ps_support_for_catalog_acls branch from c95d0e4 to 95f8feb Compare June 23, 2018 03:04

vladimir-shcherbakov added needs-review and removed Merge Conflicts needs-revision labels Jun 25, 2018

cormacpayne suggested changes Jun 25, 2018

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Jun 25, 2018

cormacpayne assigned idear1203 and unassigned cormacpayne Jun 25, 2018

Resolve comments

11a215c

idear1203 force-pushed the add_ps_support_for_catalog_acls branch from 95f8feb to 11a215c Compare June 26, 2018 02:26

cormacpayne approved these changes Jun 26, 2018

View reviewed changes

cormacpayne added Waiting for On Demand and removed Cmdlet Review Complete needs-revision labels Jun 26, 2018

cormacpayne unassigned idear1203 Jun 26, 2018

cormacpayne merged commit f97a904 into Azure:preview Jun 26, 2018

Conversation

idear1203 commented May 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Swagger PR Links

Changes

Checklist

Uh oh!

MiYanni commented Jun 6, 2018

Uh oh!

idear1203 commented Jun 8, 2018

Uh oh!

maddieclayton commented Jun 11, 2018

Uh oh!

idear1203 commented Jun 21, 2018

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

idear1203 Jun 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

idear1203 Jun 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

idear1203 Jun 22, 2018

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 22, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

idear1203 commented Jun 22, 2018

Uh oh!

idear1203 commented Jun 23, 2018

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

cormacpayne Jun 25, 2018 • edited by idear1203 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

idear1203 commented May 18, 2018 •

edited

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

idear1203 Jun 22, 2018 •

edited

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

idear1203 Jun 22, 2018 •

edited

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 22, 2018 •

edited by idear1203

Loading

cormacpayne Jun 25, 2018 •

edited by idear1203

Loading

cormacpayne Jun 25, 2018 •

edited by idear1203

Loading

cormacpayne Jun 25, 2018 •

edited by idear1203

Loading