Updates based on PR feedback for ADL

1. Move all logging related logic into the logger constructor and
dispose() methods to reduce duplication
2. Add parameter sets for diagnostic logging and make the log path
required. Also update the default log level to be Error (the least
verbose logging that actually logs, since None does not log).
3. Updated help to reflect these changes.
4. Removed parameter sets from passthru.

Author: begoldsm

src/ResourceManager/DataLakeStore/Commands.DataLakeStore/Commands/ExportAzureRmDataLakeStoreItem.cs

@@ -12,85 +12,119 @@
 // limitations under the License.
 // ----------------------------------------------------------------------------------
 
-using System.IO;
 using Microsoft.Azure.Commands.DataLakeStore.Models;
 using Microsoft.Azure.Commands.DataLakeStore.Properties;
 using Microsoft.Azure.Management.DataLake.Store.Models;
 using Microsoft.Rest.Azure;
-using System;
 using System.Management.Automation;
-using System.Net;
-using Microsoft.Rest;
 using Microsoft.IdentityModel.Clients.ActiveDirectory;
 
 namespace Microsoft.Azure.Commands.DataLakeStore
 {
-    [Cmdlet(VerbsData.Export, "AzureRmDataLakeStoreItem", SupportsShouldProcess = true), OutputType(typeof(string))]
+    [Cmdlet(VerbsData.Export, "AzureRmDataLakeStoreItem", SupportsShouldProcess = true, DefaultParameterSetName = BaseParameterSetName), OutputType(typeof(string))]
     [Alias("Export-AdlStoreItem")]
     public class ExportAzureDataLakeStoreItem : DataLakeStoreFileSystemCmdletBase
     {
+        // define parameter sets.
+        internal const string BaseParameterSetName = "No diagnostic logging";
+        internal const string DiagnosticParameterSetName = "Include diagnostic logging";
+
         // default number of threads
         private int numThreadsPerFile = 10;
         private int fileCount = 5;
-        private LogLevel logLevel = LogLevel.None;
+        private LogLevel logLevel = LogLevel.Error;
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 0, Mandatory = true,
-            HelpMessage = "The DataLakeStore account to execute the filesystem operation in")]
+            HelpMessage = "The DataLakeStore account to execute the filesystem operation in",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 0, Mandatory = true,
+            HelpMessage = "The DataLakeStore account to execute the filesystem operation in",
+            ParameterSetName = DiagnosticParameterSetName)]
         [ValidateNotNullOrEmpty]
         [Alias("AccountName")]
         public string Account { get; set; }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 1, Mandatory = true,
-            HelpMessage = "The path to the file or folder to download")]
+            HelpMessage = "The path to the file or folder to download",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 1, Mandatory = true,
+            HelpMessage = "The path to the file or folder to download",
+            ParameterSetName = DiagnosticParameterSetName)]
         [ValidateNotNull]
         public DataLakeStorePathInstance Path { get; set; }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 2, Mandatory = true,
-            HelpMessage = "The local path to download the file or folder to")]
+            HelpMessage = "The local path to download the file or folder to",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 2, Mandatory = true,
+            HelpMessage = "The local path to download the file or folder to",
+            ParameterSetName = DiagnosticParameterSetName)]
         [ValidateNotNullOrEmpty]
         public string Destination { get; set; }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 3, Mandatory = false,
-            HelpMessage = "Indicates if the download should be recursive for folder downloads. The default is false.")]
+            HelpMessage = "Indicates if the download should be recursive for folder downloads. The default is false.",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 3, Mandatory = false,
+            HelpMessage = "Indicates if the download should be recursive for folder downloads. The default is false.",
+            ParameterSetName = DiagnosticParameterSetName)]
         [ValidateNotNullOrEmpty]
         public SwitchParameter Recurse { get; set; }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 4, Mandatory = false,
             HelpMessage =
-                "Indicates that the file(s) being copied are a continuation of a previous download. This will cause the system to attempt to resume from the last file that was not fully downloaded."
-            )]
+                "Indicates that the file(s) being copied are a continuation of a previous download. This will cause the system to attempt to resume from the last file that was not fully downloaded.",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 4, Mandatory = false,
+            HelpMessage =
+                "Indicates that the file(s) being copied are a continuation of a previous download. This will cause the system to attempt to resume from the last file that was not fully downloaded.",
+            ParameterSetName = DiagnosticParameterSetName)]
         public SwitchParameter Resume { get; set; }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 5, Mandatory = false,
-            HelpMessage = "Indicates the maximum number of threads to use per file. Default is 10")]
+            HelpMessage = "Indicates the maximum number of threads to use per file. Default is 10",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 5, Mandatory = false,
+            HelpMessage = "Indicates the maximum number of threads to use per file. Default is 10",
+            ParameterSetName = DiagnosticParameterSetName)]
         public int PerFileThreadCount
         {
             get { return numThreadsPerFile; }
             set { numThreadsPerFile = value; }
         }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 6, Mandatory = false,
-            HelpMessage = "Indicates the maximum number of files to download in parallel for a folder download. Default is 5")]
+            HelpMessage = "Indicates the maximum number of files to download in parallel for a folder download. Default is 5",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 6, Mandatory = false,
+            HelpMessage = "Indicates the maximum number of files to download in parallel for a folder download. Default is 5",
+            ParameterSetName = DiagnosticParameterSetName)]
         public int ConcurrentFileCount
         {
             get { return fileCount; }
             set { fileCount = value; }
         }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Position = 7, Mandatory = false,
-            HelpMessage = "Indicates that, if the file or folder exists, it should be overwritten")]
+            HelpMessage = "Indicates that, if the file or folder exists, it should be overwritten",
+            ParameterSetName = BaseParameterSetName)]
+        [Parameter(ValueFromPipelineByPropertyName = true, Position = 7, Mandatory = false,
+            HelpMessage = "Indicates that, if the file or folder exists, it should be overwritten",
+            ParameterSetName = DiagnosticParameterSetName)]
         public SwitchParameter Force { get; set; }
 
         [Parameter(ValueFromPipelineByPropertyName = true, Mandatory = false,
-            HelpMessage = "Optionally indicates the diagnostic log level to use to record events during the file or folder import. Default is none, and specifying -Debug overwrites this value and sets it to Debug.")]
+            HelpMessage = "Optionally indicates the diagnostic log level to use to record events during the file or folder import. Default is Error.",
+            ParameterSetName = DiagnosticParameterSetName)]
         public LogLevel DiagnosticLogLevel
         {
             get { return logLevel; }
             set { logLevel = value; }
         }
 
-        [Parameter(ValueFromPipelineByPropertyName = true, Mandatory = false,
-            HelpMessage = "Optionally specifies the path for the diagnostic log to record events to during the file or folder import. If logging is enabled, the default is in %LOCALAPPDATA%\\ADLDataTransfer.")]
+        [Parameter(ValueFromPipelineByPropertyName = true, Mandatory = true,
+            HelpMessage = "Specifies the path for the diagnostic log to record events to during the file or folder import.",
+            ParameterSetName = DiagnosticParameterSetName)]
         [ValidateNotNullOrEmpty]
         public string DiagnosticLogPath { get; set; }
 
@@ -99,11 +133,6 @@ public override void ExecuteCmdlet()
             // We will let this throw itself if the path they give us is invalid
             // TODO: perhaps in the future catch this and throw a cmdlet specific exception
             var powerShellReadyPath = SessionState.Path.GetUnresolvedProviderPathFromPSPath(Destination);
-            string diagnosticPath = string.Empty;
-            if (!string.IsNullOrEmpty(DiagnosticLogPath))
-            {
-                diagnosticPath = SessionState.Path.GetUnresolvedProviderPathFromPSPath(DiagnosticLogPath);
-            }
 
             FileType type;
             ConfirmAction(
@@ -121,22 +150,12 @@ public override void ExecuteCmdlet()
                     var originalLegacyLevel = AdalTrace.LegacyTraceSwitch.Level;
                     try
                     {
-                        if (DiagnosticLogLevel != LogLevel.None)
+                        if (ParameterSetName.Equals(DiagnosticParameterSetName) && DiagnosticLogLevel != LogLevel.None)
                         {
+                            var diagnosticPath = SessionState.Path.GetUnresolvedProviderPathFromPSPath(DiagnosticLogPath);
                             logger = new DataLakeStoreTraceLogger(this, diagnosticPath, DiagnosticLogLevel);
-                            if (logger.SdkTracingInterceptor != null)
-                            {
-                                ServiceClientTracing.AddTracingInterceptor(logger.SdkTracingInterceptor);
-                            }
-
-                            // only log all ADAL messages when the level is debug.
-                            // In all other cases only log Warning and above.
-                            if (DiagnosticLogLevel != LogLevel.Debug)
-                            {
-                                AdalTrace.TraceSource.Switch.Level = System.Diagnostics.SourceLevels.Warning;
-                                AdalTrace.LegacyTraceSwitch.Level = System.Diagnostics.TraceLevel.Warning;
-                            }
                         }
+
                         if (type == FileType.FILE)
                         {
                             DataLakeStoreFileSystemClient.CopyFile(powerShellReadyPath, Account, Path.TransformedPath, CmdletCancellationToken,
@@ -156,18 +175,10 @@ public override void ExecuteCmdlet()
                     {
                         if (logger != null)
                         {
-                            if (logger.SdkTracingInterceptor != null)
-                            {
-                                ServiceClientTracing.RemoveTracingInterceptor(logger.SdkTracingInterceptor);
-                            }
-
                             // dispose and free the logger.
                             logger.Dispose();
                             logger = null;
                         }
-
-                        AdalTrace.TraceSource.Switch.Level = originalLevel;
-                        AdalTrace.LegacyTraceSwitch.Level = originalLegacyLevel;
                     }
                 });
         }