Merge pull request #601 from Azure/dev

huangpf PR: dev <- Azure:dev

Author: huangpf

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,6 +1,12 @@
+## Comments
+
+---
+
 This checklist is used to make sure that common issues in a pull request are covered by the creator. You can find a more complete discussion of PowerShell cmdlet best practices [here](https://msdn.microsoft.com/en-us/library/dd878270(v=vs.85).aspx).
 
-Below in **Overall Changes**, check off the boxes that apply to your PR. Within each of the categories that you select, make sure that you can check off **all** of the boxes.
+Below in **Overall Changes**, check off the boxes that apply to your PR. For the categories that you did not check off, you can remove them from this body. Within each of the categories that you did select, make sure that you can check off **all** of the boxes. 
+
+For information on cleaning up the commits in your pull request, click [here](../documentation/cleaning-up-commits.md).
 
 ## Overall Changes 
 - [ ] [**MANDATORY** - General changes](#general)

documentation/cleaning-up-commits.md

@@ -0,0 +1,58 @@
+## Cleaning up commits
+
+### Best practices for keeping commits clean
+
+During development, to make sure that your commit history stays clean while the branch you're based off of is changing, follow some of these rules:
+
+1. **Rebase instead of merging**
+    - When you need to update your branch with the changes made to the base branch, [rebasing](#rebasing) will change the commit that your branch is based off of, allowing you to add the changes from the base branch and not gain an extra commit that you would with merging
+2. **Large number of changes**
+    - If you create a pull request that contains a large number of changes (*e.g.,* re-recording tests) that won't be able to be displayed on GitHub, separate your changes into multiple pull requests that reference each other.
+
+### Number of commits
+
+It can be difficult to follow the changes in a pull request when the number of commits that come with it become too large:
+- If a bug fix is being addressed, a single commit should be submitted
+- If a new feature is being introduced, then the pull request can have multiple logical commits with each commit clearly describing what it does
+
+### Rebasing
+
+Sometimes a pull request can be based on a much earlier commit in the branch that you are trying to merge into it, causing a large amount of commits and file changes to litter the pull request. In this case, it would be better to **rebase** (move branches around by changing the commit that they are based on). After rebasing, you will want to close the pull request and open a new one, which will now have fewer commits.
+
+For example, if you're working from the branch **feature** and are trying to rebase with **master**, you may run one of the following commands:
+> `git rebase master`
+> `git rebase master feature`
+
+You can also rebase with the following command:
+> `git pull --rebase`
+
+A normal `git pull` is equivalent to `git fetch` followed by `git merge FETCH_HEAD`, but when you run `git pull --rebase`, it runs `git rebase` instead of `git merge`.
+
+For more information on rebasing, click [here](https://git-scm.com/docs/git-rebase).
+
+### Squashing
+
+When your pull request has a group of commits that can be condensed into one, logical commit, use **squashing**. This will clean up the number of commits your pull request has while also grouping together common commits. 
+
+For example, if you wanted to squash the last three commits into one, you may run the following command:
+> `git rebase -i HEAD~3`
+
+This will bring up an editor showing your last three commits. Pick a commit to keep (as the message), and squash the other two into it.
+
+For more information on squashing, click [here](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#Squashing-Commits).
+
+### Cherry-picking
+
+If you want to merge specific commits from another branch into the current one you are working from, use **cherry-picking**. 
+
+For example, if you're working on the **master** branch and want to pull commit X (the commit-hash) from the **feature** branch, you may run the following commands:
+> `git checkout master`
+> `git cherry-pick X -n`
+
+The `-n`, or `--no-commit`, is recommended for cherry-picking because it won't automatically create a commit for the cherry-picked change; this will allow you to view the changes first and make sure that you want to add all everything from the cherry-picked commit.
+
+Now, if you want to cherry-pick a range of commits, say X through Y, from the **feature** branch, you may run the following commands:
+> `git checkout -b temp-branch X`
+> `git rebase --onto master Y^`
+
+For more information on cherry-picking, click [here](https://git-scm.com/docs/git-cherry-pick).

src/Common/Commands.Common.Authentication/Factories/ClientFactory.cs

@@ -16,11 +16,14 @@
 using Microsoft.Azure.Commands.Common.Authentication.Models;
 using Microsoft.Azure.Commands.Common.Authentication.Properties;
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Collections.Specialized;
+using System.Linq;
 using System.Net;
 using System.Net.Http;
 using System.Net.Http.Headers;
+using System.Threading;
 
 namespace Microsoft.Azure.Commands.Common.Authentication.Factories
 {
@@ -30,12 +33,16 @@ public class ClientFactory : IClientFactory
 
         private Dictionary<Type, IClientAction> _actions;
         private OrderedDictionary _handlers;
+        private ReaderWriterLockSlim _actionsLock;
+        private ReaderWriterLockSlim _handlersLock;
 
         public ClientFactory()
         {
             _actions = new Dictionary<Type, IClientAction>();
+            _actionsLock = new ReaderWriterLockSlim();
             UserAgents = new HashSet<ProductInfoHeaderValue>();
             _handlers = new OrderedDictionary();
+            _handlersLock = new ReaderWriterLockSlim();
         }
 
         public virtual TClient CreateArmClient<TClient>(AzureContext context, AzureEnvironment.Endpoint endpoint) where TClient : Microsoft.Rest.ServiceClient<TClient>
@@ -108,8 +115,7 @@ public virtual TClient CreateClient<TClient>(AzureContext context, AzureEnvironm
         public virtual TClient CreateClient<TClient>(AzureSMProfile profile, AzureEnvironment.Endpoint endpoint) where TClient : ServiceClient<TClient>
         {
             TClient client = CreateClient<TClient>(profile.Context, endpoint);
-
-            foreach (IClientAction action in _actions.Values)
+            foreach (IClientAction action in GetActions())
             {
                 action.Apply<TClient>(client, profile, endpoint);
             }
@@ -154,7 +160,7 @@ public virtual TClient CreateClient<TClient>(AzureSMProfile profile, AzureSubscr
 
             TClient client = CreateClient<TClient>(context, endpoint);
 
-            foreach (IClientAction action in _actions.Values)
+            foreach (IClientAction action in GetActions())
             {
                 action.Apply<TClient>(client, profile, endpoint);
             }
@@ -242,34 +248,82 @@ public static HttpClientHandler CreateHttpClientHandler(string endpoint, ICreden
 
         public void AddAction(IClientAction action)
         {
-            if (action != null)
+            _actionsLock.EnterWriteLock();
+            try
+            {
+                if (action != null)
+                {
+                    action.ClientFactory = this;
+                    _actions[action.GetType()] = action;
+                }
+            }
+            finally
             {
-                action.ClientFactory = this;
-                _actions[action.GetType()] = action;
+                _actionsLock.ExitWriteLock();
             }
         }
 
         public void RemoveAction(Type actionType)
         {
-            if (_actions.ContainsKey(actionType))
+            _actionsLock.EnterWriteLock();
+            try
             {
-                _actions.Remove(actionType);
+                if (_actions.ContainsKey(actionType))
+                {
+                    _actions.Remove(actionType);
+                }
+            }
+            finally
+            {
+                _actionsLock.ExitWriteLock();
             }
         }
 
+        private IClientAction[] GetActions()
+        {
+            IClientAction[] result = null;
+            _actionsLock.EnterReadLock();
+            try
+            {
+                result = _actions.Values.ToArray();
+            }
+            finally
+            {
+                _actionsLock.ExitReadLock();
+            }
+
+            return result;
+        }
+
         public void AddHandler<T>(T handler) where T : DelegatingHandler, ICloneable
         {
-            if (handler != null)
+            _handlersLock.EnterWriteLock();
+            try
             {
-                _handlers[handler.GetType()] = handler;
+                if (handler != null)
+                {
+                    _handlers[handler.GetType()] = handler;
+                }
+            }
+            finally
+            {
+                _handlersLock.ExitWriteLock();
             }
         }
 
         public void RemoveHandler(Type handlerType)
         {
-            if (_handlers.Contains(handlerType))
+            _handlersLock.EnterWriteLock();
+            try
+            {
+                if (_handlers.Contains(handlerType))
+                {
+                    _handlers.Remove(handlerType);
+                }
+            }
+            finally
             {
-                _handlers.Remove(handlerType);
+                _handlersLock.ExitWriteLock();
             }
         }
 
@@ -296,24 +350,32 @@ public void AddUserAgent(string productName)
 
         public DelegatingHandler[] GetCustomHandlers()
         {
-            List<DelegatingHandler> newHandlers = new List<DelegatingHandler>();
-            var enumerator = _handlers.GetEnumerator();
-            while (enumerator.MoveNext())
+            _handlersLock.EnterReadLock();
+            try
             {
-                var handler = enumerator.Value;
-                ICloneable cloneableHandler = handler as ICloneable;
-                if (cloneableHandler != null)
+                List<DelegatingHandler> newHandlers = new List<DelegatingHandler>();
+                var enumerator = _handlers.GetEnumerator();
+                while (enumerator.MoveNext())
                 {
-                    var newHandler = cloneableHandler.Clone();
-                    DelegatingHandler convertedHandler = newHandler as DelegatingHandler;
-                    if (convertedHandler != null)
+                    var handler = enumerator.Value;
+                    ICloneable cloneableHandler = handler as ICloneable;
+                    if (cloneableHandler != null)
                     {
-                        newHandlers.Add(convertedHandler);
+                        var newHandler = cloneableHandler.Clone();
+                        DelegatingHandler convertedHandler = newHandler as DelegatingHandler;
+                        if (convertedHandler != null)
+                        {
+                            newHandlers.Add(convertedHandler);
+                        }
                     }
                 }
-            }
 
-            return newHandlers.ToArray();
+                return newHandlers.ToArray();
+            }
+            finally
+            {
+                _handlersLock.ExitReadLock();
+            }
         }
     }
 }

src/ResourceManager/MachineLearning/Commands.MachineLearning/Cmdlets/ExportWebServiceDefinition.cs

@@ -27,7 +27,7 @@ namespace Microsoft.Azure.Commands.MachineLearning.Cmdlets
         WebServicesCmdletBase.CommandletSuffix, 
         SupportsShouldProcess = true)]
     [OutputType(typeof(string))]
-    public class ExportWebServiceDefinition : AzureRMCmdlet
+    public class ExportWebServiceDefinition : WebServicesCmdletBase
     {
         private const string ExportToFileParamSet = "Export to file.";
         private const string ExportToStringParamSet = "Export to JSON string.";
@@ -60,7 +60,7 @@ public class ExportWebServiceDefinition : AzureRMCmdlet
             HelpMessage = "Do not ask for confirmation.")]
         public SwitchParameter Force { get; set; }
 
-        public override void ExecuteCmdlet()
+        protected override void RunCmdlet()
         {
             string serializedDefinition = 
                 ModelsSerializationUtil.GetAzureMLWebServiceDefinitionJsonFromObject(this.WebService);

src/ResourceManager/MachineLearning/Commands.MachineLearning/Cmdlets/ImportWebServiceDefinition.cs

@@ -23,7 +23,7 @@ namespace Microsoft.Azure.Commands.MachineLearning.Cmdlets
 {
     [Cmdlet(VerbsData.Import, WebServicesCmdletBase.CommandletSuffix)]
     [OutputType(typeof(WebService))]
-    public class ImportWebServiceDefinition : AzureRMCmdlet
+    public class ImportWebServiceDefinition : WebServicesCmdletBase
     {
         private const string ImportFromFileParamSet = "Import from JSON file.";
         private const string ImportFromStringParamSet = "Import from JSON string.";
@@ -42,7 +42,7 @@ public class ImportWebServiceDefinition : AzureRMCmdlet
         [ValidateNotNullOrEmpty]
         public string JsonString { get; set; }
 
-        public override void ExecuteCmdlet()
+        protected override void RunCmdlet()
         {
             string jsonDefinition = this.JsonString;
             if (string.Equals(

src/ResourceManager/MachineLearning/Commands.MachineLearning/Cmdlets/WebServicesCmdletBase.cs

@@ -82,6 +82,7 @@ protected override void BeginProcessing()
             }
             catch (Exception ex)
             {
+                this.WriteVersionInfoToDebugChannel();
                 if (this.IsFatalException(ex))
                 {
                     ThrowTerminatingError(
@@ -105,6 +106,7 @@ protected override void EndProcessing()
             }
             catch (Exception ex)
             {
+                this.WriteVersionInfoToDebugChannel();
                 if (this.IsFatalException(ex))
                 {
                     ThrowTerminatingError(
@@ -138,6 +140,7 @@ protected override void StopProcessing()
             }
             catch (Exception ex)
             {
+                this.WriteVersionInfoToDebugChannel();
                 if (this.IsFatalException(ex))
                 {
                     throw;
@@ -169,6 +172,8 @@ public override void ExecuteCmdlet()
             }
             catch (Exception ex)
             {
+               this.WriteVersionInfoToDebugChannel();
+
                 if (this.IsFatalException(ex))
                 {
                     throw;
@@ -329,5 +334,10 @@ ex is SecurityException ||
                 ex is SEHException;
         }
 
+        private void WriteVersionInfoToDebugChannel()
+        {
+            var versionInfo = this.MyInvocation.MyCommand.Module.Version;
+            this.WriteDebug(Resources.VersionInfo.FormatInvariant(versionInfo.ToString(3)));
+        }
     }
 }

src/ResourceManager/MachineLearning/Commands.MachineLearning/Resources.Designer.cs

@@ -135,4 +135,7 @@
   <data name="UpdateServiceWarning" xml:space="preserve">
     <value>Are you sure you want to update the machine learning web service "{0}" ?</value>
   </data>
+  <data name="VersionInfo" xml:space="preserve">
+    <value>Azure Machine Learning Module Version: "{0}"</value>
+  </data>
 </root>