-
Notifications
You must be signed in to change notification settings - Fork 4k
Support creating Spark 2.0 cluster using Azure HDInsight PowerShell #3110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
87ea3c5
6ccc667
4952670
245e738
056de9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
// ---------------------------------------------------------------------------------- | ||
// | ||
// Copyright Microsoft Corporation | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
// ---------------------------------------------------------------------------------- | ||
|
||
using Microsoft.Azure.Commands.HDInsight.Commands; | ||
using Microsoft.Azure.Commands.HDInsight.Models; | ||
using System.Management.Automation; | ||
|
||
namespace Microsoft.Azure.Commands.HDInsight | ||
{ | ||
[Cmdlet( | ||
VerbsCommon.Add, | ||
Constants.CommandNames.AzureHDInsightComponentVersion, | ||
SupportsShouldProcess = true), | ||
OutputType(typeof(AzureHDInsightConfig))] | ||
public class AddAzureHDInsightComponentVersionCommand : HDInsightCmdletBase | ||
{ | ||
#region Input Parameter Definitions | ||
|
||
[Parameter(Position = 0, | ||
Mandatory = true, | ||
ValueFromPipeline = true, | ||
HelpMessage = "The HDInsight cluster configuration to use when creating the new cluster.")] | ||
public AzureHDInsightConfig Config { get; set; } | ||
|
||
[Parameter(Position = 1, | ||
Mandatory = true, | ||
HelpMessage = "The component name whose version should be specified for the new cluster.")] | ||
public string ComponentName { get; set; } | ||
|
||
[Parameter(Position = 2, | ||
Mandatory = true, | ||
HelpMessage = "The version of a specific service in the new cluster.")] | ||
public string ComponentVersion { get; set; } | ||
|
||
#endregion | ||
|
||
public override void ExecuteCmdlet() | ||
{ | ||
ConfirmAction("Adding Component Version", "AzureHDInsightConfig", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be 'ComponentName'? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, the resource being modified is the AzureHDInsightConfig because there's no ComponentName object There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
() => | ||
{ | ||
Config.ComponentVersion.Add(ComponentName, ComponentVersion); | ||
WriteObject(Config); | ||
}); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is not all. You also need to either call ConfirmAction or ShoudlProcess correctly during cmdlet execution. See: https://gist.github.com/markcowl/338e16fe5c8bbf195aff9f8af0db585d
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the PR with a call to ConfirmAction from the cmdlet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any more comments on this change?
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markcowl ping