Integrating the new custom dataplane sdk for ADLS to the powershell cmdlets #4986

rahuldutta90 · 2017-11-14T20:54:27Z

Description

We have introduced a new custom dataplane SDK for ADLS. This new SDK replaces the dataplane part of the previous SDK. This change is due to integration of the new SDK to the current powershell cmdlets.

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request 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 more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

…shell into preview

…eprecated ConcurrentFileCount parameter and introduced Concurrency in Import-AzureRmDataLakeStore and Export-AzureDataLakeStoreItem 3) Updated help markdown files

…nd Set-AzureRmDataLakeStoreItemExpiry

…) Marked obsolete for parameters that we are going to deprecate 3) Undo the introduction of parameter IfFileExists for New-Item 4) Introduce Mock unittest for playback

markcowl · 2017-11-15T00:54:21Z

@rahuldutta90 Please pull the latest from the branch, there are merge conflicts

… for deprecated parameters

rahuldutta90 · 2017-11-15T04:19:20Z

@markcowl Did a latest pull from the main repo and resolved the conflicts.

maddieclayton · 2017-11-20T18:05:57Z

@rahuldutta90 You are currently failing the build because of Signature issues - PowerShell generally disapproves of the use of plural parameter names (FileExpiryOptions). If this has been expressly approved in review (I do not think I was in attendance for this review), just add the error (found in the build under view/src/Package/SignatureIssues.csv with error code 1) to the SignatureIssue.csv file here

rahuldutta90 · 2017-11-21T17:26:15Z

@maddieclayton I have fixed parameter name FileExpiryOptions. Thanks.

cormacpayne

@rahuldutta90 a few comments to take a look at

cormacpayne · 2017-11-22T01:22:58Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/Commands.DataLakeStore.csproj

    <Reference Include="Microsoft.Azure.Management.DataLake.Store, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
      <SpecificVersion>False</SpecificVersion>
      <HintPath>..\..\..\packages\Microsoft.Azure.Management.DataLake.Store.2.3.0-preview\lib\net452\Microsoft.Azure.Management.DataLake.Store.dll</HintPath>
      <Private>True</Private>
    </Reference>
+    <Reference Include="NLog, Version=4.0.0.0, Culture=neutral, PublicKeyToken=5120e14c03d0593c, processorArchitecture=MSIL">


@rahuldutta90 what is this package being used for?

This 3rd party package is used for logging. Without this it wont run.

Has this been through IP review?

@rahuldutta90 ping on this comment

@cormacpayne NLog is OSS registered package used for logging.

cormacpayne · 2017-11-22T01:23:55Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/Commands.DataLakeStore.csproj

@@ -116,7 +126,15 @@
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </None>
    <None Include="MSSharedLibKey.snk" />
-    <None Include="packages.config" />
+    <Content Include="NLog.config">


@rahuldutta90 what are NLog.config and NLog.xsd being used for?

This is not mandatory. It is used to configure NLog. I had a dependency on NLog.config which was not needed. For future release I have removed this dependency. Here I have manually removed these files, updated the csproj and packages.config

cormacpayne · 2017-11-22T01:24:38Z

...ger/DataLakeStore/Commands.DataLakeStore/DataPlaneCommands/ExportAzureRmDataLakeStoreItem.cs

@@ -81,29 +76,25 @@ public class ExportAzureDataLakeStoreItem : DataLakeStoreFileSystemCmdletBase
            ParameterSetName = DiagnosticParameterSetName)]
        public SwitchParameter Resume { get; set; }

+        [Obsolete]


@rahuldutta90 for all Obsolete attributes, please make sure you include a message that says what parameter you are deprecating in an upcoming breaking change release and what cmdlet it is in.

Fixed. Also fixed this for ImportAzureRmDataLakeStoreItem and RemoveAzureDataLakeStoreItem.

cormacpayne · 2017-11-22T01:26:09Z

...ger/DataLakeStore/Commands.DataLakeStore/DataPlaneCommands/ExportAzureRmDataLakeStoreItem.cs

@@ -113,14 +104,21 @@ public int ConcurrentFileCount
            ParameterSetName = DiagnosticParameterSetName)]
        public SwitchParameter Force { get; set; }

+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 8, Mandatory = false,


@rahuldutta90 please remove the Position from both of the parameter attributes you are adding here

Fixed. Updated the markdown file also. Also did the same fix for ImportAzureDataLakeStoreItem since I introduced the same parameter there.

cormacpayne · 2017-11-22T01:27:26Z

...ger/DataLakeStore/Commands.DataLakeStore/DataPlaneCommands/ExportAzureRmDataLakeStoreItem.cs

@@ -130,63 +128,55 @@ public LogLevel DiagnosticLogLevel

        public override void ExecuteCmdlet()
        {
+            if (ConcurrentFileCount != -1)
+            {
+                WriteWarning(Resources.IncorrectConcurrentFileCountWarning);


@rahuldutta90 rather than writing this warning, put these strings inside their corresponding Obsolete attribute

Still not sure. Although updated the obsolete message, but I do not see the warning when I remove WriteWarning even though I have the message in obsolete.

@cormacpayne Do you have any other comments regarding this?

@cormacpayne , I have maked obsolete. However, that alone does not show the message when running the cmdlet. So I am leaving the WriteWarning in as well.

cormacpayne · 2017-11-22T01:33:57Z

.../DataLakeStore/Commands.DataLakeStore/DataPlaneCommands/SetAzureRmDataLakeStoreItemExpiry.cs

+    [Alias("Set-AdlStoreItemExpiry")]
+    public class SetAzureRmDataLakeStoreItemExpiry : DataLakeStoreFileSystemCmdletBase
+    {
+        internal const string BaseParameterSetName = "Set expiry as Absolute or NeverExpire";


@rahuldutta90 make sure parameter set names are not space-separated strings

Fixed. Updated the markdown file also.

cormacpayne · 2017-11-22T01:34:28Z

.../DataLakeStore/Commands.DataLakeStore/DataPlaneCommands/SetAzureRmDataLakeStoreItemExpiry.cs

+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 3, ParameterSetName = RelativeExpiryParameterSetName, Mandatory = false,
+            HelpMessage = "The relative time in milliseconds with respect to now or creation time")]
+        [ValidateNotNullOrEmpty]
+        public long? RelativeTime { get; set; }


@rahuldutta90 nit: make sure there's an empty line between this parameter and ExecuteCmdlet()

cormacpayne · 2017-11-22T01:37:40Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/DataPlaneModels/DataLakeStoreItem.cs

+
+        public long? AccessTime { get; }
+        /// <summary>Gets the block size for the file.</summary>
+        public long? BlockSize { get; }


@rahuldutta90 nit: would you mind adding an empty line between each property?

cormacpayne · 2017-11-22T01:40:05Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/help/Move-AzureRmDataLakeStoreItem.md

-The credentials, account, tenant, and subscription used for communication with azure
-
-```yaml
+The credentials, account, tenant, and subscription used for communication with azure.```yaml


@rahuldutta90 this is a bug in the platyPS module - please make sure that you move the ```yaml string at the end of this line to a line by itself. You can see how this should be done by looking at any other parameter in this file.

Good catch. This one I missed.

…rameter set, specify message in Obsolete

rahuldutta90 · 2017-11-27T18:02:08Z

@cormacpayne I have responded to your reviews.

cormacpayne · 2017-11-30T19:48:37Z

@azuresdkci test this please

shawns1 · 2017-12-01T06:18:52Z

@azuresdkci test this please

…ameter mandatory in SetExpiry

…tExpiry

markcowl · 2017-12-04T20:16:15Z

Per email, will create a branch for this after the release candidate goes out to release as a preview.

rahuldutta90 · 2017-12-11T17:58:39Z

@markcowl Azure powershell 5.1.1 is released. Now what do we have to do to release this out of band? Do we have the separate branch?

maddieclayton · 2017-12-14T21:42:47Z

@azuresdkci Test this please

markcowl · 2017-12-15T00:57:57Z

on demand run here:https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/286/

signign run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-sign/56/

cormacpayne

@rahuldutta90 one comment to resolve. To prepare your module to be preview, you will need to do the following:

(1) You will need to update your version of PowerShellGet to 1.6.0, which you can do by running Install-Module -Name PowerShellGet -Repository PSGallery -Force

(2) Update the ModuleVersion in AzureRM.DataLakeStore.psd1 to 5.0.1

(3) Update the ReleaseNotes field in AzureRM.DataLakeStore.psd1 with the contents that you added to your ChangeLog.md file (the contents underneath the ## Current Release header)

(4) Update the Prelease field in AzureRM.DataLakeStore.psd1 to preview. You can do this by running Update-ModuleManifest -Path PATH_TO_PSD1 -Prelease "preview"

(5) Update the versions in AssemblyInfo.cs to 5.0.1

cormacpayne · 2017-12-15T19:03:59Z

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/packages.config

  <package id="Microsoft.Azure.Management.DataLake.Store" version="2.3.0-preview" targetFramework="net452" />
+  <package id="NLog" version="4.4.12" targetFramework="net452" />


@rahuldutta90 since we've confirmed that these packages are OK to use, you will need to add them to the RequiredAssemblies field in the AzureRM.DataLakeStore.psd1 file.

Will update module versions out of band

rahuldutta90 added 9 commits October 12, 2017 18:31

Replaced the old filesystem sdk with the current adls dotnetsdk

b2ea51e

Fixes to the warning messages, Documented changes and breaking changes

4e6c491

Updated the markdown files

7581898

Merge branch 'preview' into preview

38a2a72

Fix the markdown file for NEw-AzureRmDataLakeStoreItem

26253c1

Merge branch 'preview' of https://github.com/rahuldutta90/azure-power…

c12f7dc

…shell into preview

1) Deprecated clean parameter in Remove-AzureRmDataLakeStoreItem 2) D…

655d3e5

…eprecated ConcurrentFileCount parameter and introduced Concurrency in Import-AzureRmDataLakeStore and Export-AzureDataLakeStoreItem 3) Updated help markdown files

Updated breaking changes, examples for New-AzureRmDataLakeStoreItem a…

b950e87

…nd Set-AzureRmDataLakeStoreItemExpiry

1) Restored parameters resume and forcebinary for Import and export 2…

bdadfae

…) Marked obsolete for parameters that we are going to deprecate 3) Undo the introduction of parameter IfFileExists for New-Item 4) Introduce Mock unittest for playback

rahuldutta90 self-assigned this Nov 14, 2017

rahuldutta90 requested review from markcowl, cormacpayne and asikaria-msft November 14, 2017 20:54

azuresdkci added the needs-review label Nov 14, 2017

markcowl added Do Not Merge 🚫 Cmdlet Review Required ⚠️ labels Nov 14, 2017

rahuldutta90 added 2 commits November 14, 2017 18:23

Merge from https://github.com/Azure/azure-powershell.git

201ab30

1) Resolve the merge for parametersetname 2) Restore the help message…

4d717fe

… for deprecated parameters

Add management reference

0a6c68c

Make the parameter for setExpiry singular

d3cfbf5

cormacpayne suggested changes Nov 22, 2017

View reviewed changes

Remove unecessary dependencies like NLog.config, remove space from pa…

b5245f0

…rameter set, specify message in Obsolete

cormacpayne assigned cormacpayne and unassigned rahuldutta90 Nov 30, 2017

markcowl added needs-revision and removed needs-review labels Dec 1, 2017

markcowl assigned rahuldutta90 and unassigned cormacpayne Dec 1, 2017

rahuldutta90 added 2 commits December 1, 2017 11:13

Add build exceptions, Remove nullable type from parameter, MAke a par…

1b72d24

…ameter mandatory in SetExpiry

Added exception for Changing type of Expiration to non nullable in Se…

c3ca135

…tExpiry

markcowl self-assigned this Dec 4, 2017

markcowl added the Do Not Merge 🚫 label Dec 4, 2017

rahuldutta90 closed this Dec 11, 2017

cormacpayne reopened this Dec 14, 2017

cormacpayne changed the base branch from preview to adls-data-plane December 14, 2017 17:11

cormacpayne added 2 commits December 14, 2017 09:13

Merge branch 'adls-data-plane' into preview

e2360f9

Merge branch 'adls-data-plane' into preview

ad92197

cormacpayne unassigned rahuldutta90 Dec 14, 2017

cormacpayne added needs-review Preview and removed Do Not Merge 🚫 needs-revision labels Dec 14, 2017

markcowl approved these changes Dec 15, 2017

View reviewed changes

markcowl added Waiting for On Demand and removed needs-review labels Dec 15, 2017

cormacpayne previously requested changes Dec 15, 2017

View reviewed changes

markcowl merged commit d003249 into Azure:adls-data-plane Dec 16, 2017

rahuldutta90 mentioned this pull request Dec 18, 2017

Update AzureRM.DataLakeStore.psd1 and AssemblyInfo.cs #5160

Merged

13 tasks

		<package id="Microsoft.Azure.Management.DataLake.Store" version="2.3.0-preview" targetFramework="net452" />
		<package id="NLog" version="4.4.12" targetFramework="net452" />

Integrating the new custom dataplane sdk for ADLS to the powershell cmdlets #4986

Integrating the new custom dataplane sdk for ADLS to the powershell cmdlets #4986

Uh oh!

Conversation

rahuldutta90 commented Nov 14, 2017

Description

We have introduced a new custom dataplane SDK for ADLS. This new SDK replaces the dataplane part of the previous SDK. This change is due to integration of the new SDK to the current powershell cmdlets.

Uh oh!

markcowl commented Nov 15, 2017

Uh oh!

rahuldutta90 commented Nov 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maddieclayton commented Nov 20, 2017

Uh oh!

rahuldutta90 commented Nov 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rahuldutta90 Nov 22, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rahuldutta90 Nov 22, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

rahuldutta90 Nov 22, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

rahuldutta90 Nov 22, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rahuldutta90 commented Nov 27, 2017

Uh oh!

cormacpayne commented Nov 30, 2017

Uh oh!

shawns1 commented Dec 1, 2017

rahuldutta90 commented Nov 15, 2017 •

edited

Loading

rahuldutta90 commented Nov 21, 2017 •

edited

Loading

rahuldutta90 Nov 22, 2017 •

edited

Loading

rahuldutta90 Nov 22, 2017 •

edited

Loading

rahuldutta90 Nov 22, 2017 •

edited

Loading

rahuldutta90 Nov 22, 2017 •

edited

Loading

markcowl commented Dec 15, 2017 •

edited

Loading