From ac2e13193dea424a6ca25eb0759843b4a1829c79 Mon Sep 17 00:00:00 2001 From: Matt Galbraith Date: Wed, 12 May 2021 17:25:21 -0700 Subject: [PATCH] Add CLI options to support failure notification tagging and plumb the new argument through to the generated client. --- .../src/Generated/Models/SubscriptionData.cs | 3 ++- .../Maestro.DataProviders/MaestroBarClient.cs | 2 +- .../MaestroBarClient.cs | 2 +- .../src/Darc/Helpers/UxHelpers.cs | 1 + .../Models/PopUps/AddSubscriptionPopUp.cs | 17 ++++++++++++++--- .../src/Darc/Models/PopUps/EditorPopUp.cs | 2 +- .../Models/PopUps/UpdateSubscriptionPopUp.cs | 12 +++++++++++- .../Operations/AddSubscriptionOperation.cs | 19 +++++++++++++++++-- .../Operations/UpdateSubscriptionOperation.cs | 5 ++++- .../AddSubscriptionCommandLineOptions.cs | 9 ++++++--- .../src/DarcLib/Actions/Remote.cs | 6 ++++-- .../src/DarcLib/IBarClient.cs | 3 ++- .../src/DarcLib/IRemote.cs | 3 ++- .../src/DarcLib/MaestroApiBarClient.cs | 5 +++-- 14 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/Maestro/Client/src/Generated/Models/SubscriptionData.cs b/src/Maestro/Client/src/Generated/Models/SubscriptionData.cs index 1e9349d70a..8388ad9059 100644 --- a/src/Maestro/Client/src/Generated/Models/SubscriptionData.cs +++ b/src/Maestro/Client/src/Generated/Models/SubscriptionData.cs @@ -6,13 +6,14 @@ namespace Microsoft.DotNet.Maestro.Client.Models { public partial class SubscriptionData { - public SubscriptionData(string channelName, string sourceRepository, string targetRepository, string targetBranch, Models.SubscriptionPolicy policy) + public SubscriptionData(string channelName, string sourceRepository, string targetRepository, string targetBranch, Models.SubscriptionPolicy policy, string failureNotificationTags) { ChannelName = channelName; SourceRepository = sourceRepository; TargetRepository = targetRepository; TargetBranch = targetBranch; Policy = policy; + PullRequestFailureNotificationTags = failureNotificationTags; } [JsonProperty("channelName")] diff --git a/src/Maestro/Maestro.DataProviders/MaestroBarClient.cs b/src/Maestro/Maestro.DataProviders/MaestroBarClient.cs index 43a468eebe..4da0a30099 100644 --- a/src/Maestro/Maestro.DataProviders/MaestroBarClient.cs +++ b/src/Maestro/Maestro.DataProviders/MaestroBarClient.cs @@ -52,7 +52,7 @@ public Task CreateChannelAsync(string name, string classification) } public Task CreateSubscriptionAsync(string channelName, string sourceRepo, string targetRepo, string targetBranch, - string updateFrequency, bool batchable, List mergePolicies) + string updateFrequency, bool batchable, List mergePolicies, string failureNotificationTags) { throw new NotImplementedException(); } diff --git a/src/Maestro/SubscriptionActorService/MaestroBarClient.cs b/src/Maestro/SubscriptionActorService/MaestroBarClient.cs index e600f3371e..8fdc2b7bba 100644 --- a/src/Maestro/SubscriptionActorService/MaestroBarClient.cs +++ b/src/Maestro/SubscriptionActorService/MaestroBarClient.cs @@ -42,7 +42,7 @@ public Task CreateChannelAsync(string name, string classification) } public Task CreateSubscriptionAsync(string channelName, string sourceRepo, string targetRepo, string targetBranch, - string updateFrequency, bool batchable, List mergePolicies) + string updateFrequency, bool batchable, List mergePolicies, string failureNotificationTags) { throw new NotImplementedException(); } diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Helpers/UxHelpers.cs b/src/Microsoft.DotNet.Darc/src/Darc/Helpers/UxHelpers.cs index 6a3a73644f..c2f6e1d5b2 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Helpers/UxHelpers.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Helpers/UxHelpers.cs @@ -206,6 +206,7 @@ public static string GetTextSubscriptionDescription(Subscription subscription, I subInfo.AppendLine($" - Update Frequency: {subscription.Policy.UpdateFrequency}"); subInfo.AppendLine($" - Enabled: {subscription.Enabled}"); subInfo.AppendLine($" - Batchable: {subscription.Policy.Batchable}"); + subInfo.AppendLine($" - PR Failure Notification tags: {subscription.PullRequestFailureNotificationTags}"); IEnumerable policies = mergePolicies ?? subscription.Policy.MergePolicies; subInfo.Append(UxHelpers.GetMergePoliciesDescription(policies, " ")); diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/AddSubscriptionPopUp.cs b/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/AddSubscriptionPopUp.cs index d2ce66b28f..609bf9832d 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/AddSubscriptionPopUp.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/AddSubscriptionPopUp.cs @@ -24,6 +24,7 @@ public class AddSubscriptionPopUp : EditorPopUp public string UpdateFrequency => _yamlData.UpdateFrequency; public List MergePolicies => MergePoliciesPopUpHelpers.ConvertMergePolicies(_yamlData.MergePolicies); public bool Batchable => bool.Parse(_yamlData.Batchable); + public string FailureNotificationTags => _yamlData.FailureNotificationTags; public AddSubscriptionPopUp(string path, ILogger logger, @@ -37,7 +38,8 @@ public AddSubscriptionPopUp(string path, IEnumerable suggestedChannels, IEnumerable suggestedRepositories, IEnumerable availableUpdateFrequencies, - IEnumerable availableMergePolicyHelp) + IEnumerable availableMergePolicyHelp, + string failureNotificationTags) : base(path) { _logger = logger; @@ -49,7 +51,8 @@ public AddSubscriptionPopUp(string path, TargetBranch = GetCurrentSettingForDisplay(targetBranch, "", false), UpdateFrequency = GetCurrentSettingForDisplay(updateFrequency, $"<'{string.Join("', '", Constants.AvailableFrequencies)}'>", false), Batchable = GetCurrentSettingForDisplay(batchable.ToString(), batchable.ToString(), false), - MergePolicies = MergePoliciesPopUpHelpers.ConvertMergePolicies(mergePolicies) + MergePolicies = MergePoliciesPopUpHelpers.ConvertMergePolicies(mergePolicies), + FailureNotificationTags = failureNotificationTags }; ISerializer serializer = new SerializerBuilder().Build(); @@ -63,7 +66,9 @@ public AddSubscriptionPopUp(string path, new Line("A subscription maps a build of a source repository that has been applied to a specific channel", true), new Line("onto a specific branch in a target repository. The subscription has a trigger (update frequency)", true), new Line("and merge policy. If a subscription is batchable, no merge policy should be provided, and the", true), - new Line("set-repository-policies command should be used instead to set policies on the repository + branch level. ", true), + new Line("set-repository-policies command should be used instead to set policies at the repository and branch level. ", true), + new Line("For non-batched subscriptions, providing a list of semicolon-delineated GitHub tags will tag these", true), + new Line("logins when monitoring the pull requests, once one or more policy checks fail.", true), new Line("For additional information about subscriptions, please see", true), new Line("https://github.com/dotnet/arcade/blob/main/Documentation/BranchesChannelsAndSubscriptions.md", true), new Line("", true), @@ -151,6 +156,8 @@ public override int ProcessContents(IList contents) _yamlData.Batchable = outputYamlData.Batchable; + _yamlData.FailureNotificationTags = outputYamlData.FailureNotificationTags; + _yamlData.UpdateFrequency = ParseSetting(outputYamlData.UpdateFrequency, _yamlData.UpdateFrequency, false); if (string.IsNullOrEmpty(_yamlData.UpdateFrequency) || !Constants.AvailableFrequencies.Contains(_yamlData.UpdateFrequency, StringComparer.OrdinalIgnoreCase)) @@ -176,6 +183,7 @@ class SubscriptionData public const string updateFrequencyElement = "Update Frequency"; public const string mergePolicyElement = "Merge Policies"; public const string batchableElement = "Batchable"; + public const string failureNotificationTagsElement = "Pull Request Failure Notification Tags"; [YamlMember(Alias = channelElement, ApplyNamingConventions = false)] public string Channel { get; set; } @@ -197,6 +205,9 @@ class SubscriptionData [YamlMember(Alias = mergePolicyElement, ApplyNamingConventions = false)] public List MergePolicies { get; set; } + + [YamlMember(Alias = failureNotificationTagsElement, ApplyNamingConventions = false)] + public string FailureNotificationTags { get; set; } } } } diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/EditorPopUp.cs b/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/EditorPopUp.cs index 9f4661674d..1f14d44331 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/EditorPopUp.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/EditorPopUp.cs @@ -55,7 +55,7 @@ private List GetContentValues(IEnumerable contents) /// /// Current value of the setting /// Default value if the current setting value is empty - /// If secret and current value is empty, should display *** + /// If secret and current value is not empty, should display *** /// String to display protected static string GetCurrentSettingForDisplay(string currentValue, string defaultValue, bool isSecret) { diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs b/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs index ff2ae35c54..8b1fe2bf58 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Models/PopUps/UpdateSubscriptionPopUp.cs @@ -31,6 +31,8 @@ public class UpdateSubscriptionPopUp : EditorPopUp public bool Enabled => bool.Parse(_yamlData.Enabled); + public string FailureNotificationTags => _yamlData.FailureNotificationTags; + public List MergePolicies => MergePoliciesPopUpHelpers.ConvertMergePolicies(_yamlData.MergePolicies); public UpdateSubscriptionPopUp(string path, @@ -39,7 +41,8 @@ public UpdateSubscriptionPopUp(string path, IEnumerable suggestedChannels, IEnumerable suggestedRepositories, IEnumerable availableUpdateFrequencies, - IEnumerable availableMergePolicyHelp) + IEnumerable availableMergePolicyHelp, + string failureNotificationTags) : base(path) { _logger = logger; @@ -52,6 +55,7 @@ public UpdateSubscriptionPopUp(string path, Batchable = GetCurrentSettingForDisplay(subscription.Policy.Batchable.ToString(), subscription.Policy.Batchable.ToString(), false), UpdateFrequency = GetCurrentSettingForDisplay(subscription.Policy.UpdateFrequency.ToString(), subscription.Policy.UpdateFrequency.ToString(), false), Enabled = GetCurrentSettingForDisplay(subscription.Enabled.ToString(), subscription.Enabled.ToString(), false), + FailureNotificationTags = GetCurrentSettingForDisplay(failureNotificationTags, failureNotificationTags, false) }; _yamlData.MergePolicies = MergePoliciesPopUpHelpers.ConvertMergePolicies(subscription.Policy.MergePolicies); @@ -157,6 +161,8 @@ public override int ProcessContents(IList contents) return Constants.ErrorCode; } + _yamlData.FailureNotificationTags = ParseSetting(outputYamlData.FailureNotificationTags, _yamlData.FailureNotificationTags, false); + return Constants.SuccessCode; } @@ -172,6 +178,7 @@ private class SubscriptionData public const string updateFrequencyElement = "Update Frequency"; public const string mergePolicyElement = "Merge Policies"; public const string enabled = "Enabled"; + public const string failureNotificationTagsElement = "Pull Request Failure Notification Tags"; [YamlMember(ApplyNamingConventions = false)] public string Id { get; set; } @@ -193,6 +200,9 @@ private class SubscriptionData [YamlMember(Alias = mergePolicyElement, ApplyNamingConventions = false)] public List MergePolicies { get; set; } + + [YamlMember(Alias = failureNotificationTagsElement, ApplyNamingConventions = false)] + public string FailureNotificationTags { get; set; } } } } diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Operations/AddSubscriptionOperation.cs b/src/Microsoft.DotNet.Darc/src/Darc/Operations/AddSubscriptionOperation.cs index 454ce2293e..f78fdd624a 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Operations/AddSubscriptionOperation.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Operations/AddSubscriptionOperation.cs @@ -99,6 +99,7 @@ public override async Task ExecuteAsync() string targetBranch = GitHelpers.NormalizeBranchName(_options.TargetBranch); string updateFrequency = _options.UpdateFrequency; bool batchable = _options.Batchable; + string failureNotificationTags = _options.PullRequestFailureNotificationTags; // If in quiet (non-interactive mode), ensure that all options were passed, then // just call the remote API @@ -117,6 +118,11 @@ public override async Task ExecuteAsync() } else { + if (!string.IsNullOrEmpty(failureNotificationTags) && batchable) + { + Logger.LogWarning("Failure Notification Tags may be set, but will not be used while in batched mode."); + } + // Grab existing subscriptions to get suggested values. // TODO: When this becomes paged, set a max number of results to avoid // pulling too much. @@ -138,7 +144,8 @@ public override async Task ExecuteAsync() (await suggestedChannels).Select(suggestedChannel => suggestedChannel.Name), (await suggestedRepos).SelectMany(subscription => new List {subscription.SourceRepository, subscription.TargetRepository }).ToHashSet(), Constants.AvailableFrequencies, - Constants.AvailableMergePolicyYamlHelp); + Constants.AvailableMergePolicyYamlHelp, + failureNotificationTags); UxManager uxManager = new UxManager(_options.GitLocation, Logger); int exitCode = _options.ReadStandardIn ? uxManager.ReadFromStdIn(addSubscriptionPopup) : uxManager.PopUp(addSubscriptionPopup); @@ -153,6 +160,7 @@ public override async Task ExecuteAsync() updateFrequency = addSubscriptionPopup.UpdateFrequency; mergePolicies = addSubscriptionPopup.MergePolicies; batchable = addSubscriptionPopup.Batchable; + failureNotificationTags = addSubscriptionPopup.FailureNotificationTags; } try @@ -169,6 +177,11 @@ public override async Task ExecuteAsync() Console.WriteLine($"Please use 'darc set-repository-policies --repo {targetRepository} --branch {targetBranch}' " + $"to set policies.{Environment.NewLine}"); } + + if (!string.IsNullOrEmpty(failureNotificationTags)) + { + Console.WriteLine("Warning: Failure notification tags may be set, but are ignored on batched subscriptions."); + } } // Verify the target @@ -193,7 +206,9 @@ public override async Task ExecuteAsync() targetBranch, updateFrequency, batchable, - mergePolicies); + mergePolicies, + failureNotificationTags); + Console.WriteLine($"Successfully created new subscription with id '{newSubscription.Id}'."); // Prompt the user to trigger the subscription unless they have explicitly disallowed it diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Operations/UpdateSubscriptionOperation.cs b/src/Microsoft.DotNet.Darc/src/Darc/Operations/UpdateSubscriptionOperation.cs index fc66d54d9f..8a98f420e9 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Operations/UpdateSubscriptionOperation.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Operations/UpdateSubscriptionOperation.cs @@ -49,7 +49,8 @@ public override async Task ExecuteAsync() (await suggestedChannels).Select(suggestedChannel => suggestedChannel.Name), (await suggestedRepos).SelectMany(subs => new List { subscription.SourceRepository, subscription.TargetRepository }).ToHashSet(), Constants.AvailableFrequencies, - Constants.AvailableMergePolicyYamlHelp); + Constants.AvailableMergePolicyYamlHelp, + subscription.PullRequestFailureNotificationTags); UxManager uxManager = new UxManager(_options.GitLocation, Logger); @@ -65,6 +66,7 @@ public override async Task ExecuteAsync() string updateFrequency = updateSubscriptionPopUp.UpdateFrequency; bool batchable = updateSubscriptionPopUp.Batchable; bool enabled = updateSubscriptionPopUp.Enabled; + string failureNotificationTags = updateSubscriptionPopUp.FailureNotificationTags; List mergePolicies = updateSubscriptionPopUp.MergePolicies; try @@ -75,6 +77,7 @@ public override async Task ExecuteAsync() SourceRepository = sourceRepository ?? subscription.SourceRepository, Enabled = enabled, Policy = subscription.Policy, + PullRequestFailureNotificationTags = failureNotificationTags }; subscriptionToUpdate.Policy.Batchable = batchable; subscriptionToUpdate.Policy.UpdateFrequency = Enum.Parse(updateFrequency); diff --git a/src/Microsoft.DotNet.Darc/src/Darc/Options/AddSubscriptionCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/src/Darc/Options/AddSubscriptionCommandLineOptions.cs index 035e379a20..030cb1570b 100644 --- a/src/Microsoft.DotNet.Darc/src/Darc/Options/AddSubscriptionCommandLineOptions.cs +++ b/src/Microsoft.DotNet.Darc/src/Darc/Options/AddSubscriptionCommandLineOptions.cs @@ -29,12 +29,12 @@ class AddSubscriptionCommandLineOptions : CommandLineOptions [Option("batchable", HelpText = "Make subscription batchable.")] public bool Batchable { get; set; } - [Option("standard-automerge", HelpText = "Use standard auto-merge policies. GitHub ignores WIP, license/cla and auto-merge.config.enforce checks," + + [Option("standard-automerge", HelpText = "Use standard auto-merge policies. GitHub ignores WIP, license/cla and auto-merge.config.enforce checks, " + "Azure DevOps ignores comment, reviewer and work item linking. Both will not auto-merge if changes are requested.")] public bool StandardAutoMergePolicies { get; set; } - [Option("all-checks-passed", HelpText = "PR is automatically merged if there is at least one checks and all are passed. " + - "Optionally provide a comma separated list of ignored check with --ignore-checks.")] + [Option("all-checks-passed", HelpText = "PR is automatically merged if there is at least one check, and all checks have passed. " + + "Optionally provide a comma-separated list of ignored check with --ignore-checks.")] public bool AllChecksSuccessfulMergePolicy { get; set; } [Option("ignore-checks", Separator = ',', HelpText = "For use with --all-checks-passed. A set of checks that are ignored.")] @@ -58,6 +58,9 @@ class AddSubscriptionCommandLineOptions : CommandLineOptions [Option("no-trigger", SetName = "notrigger", HelpText = "Do not trigger the subscription on creation.")] public bool NoTriggerOnCreate { get; set; } + [Option("failure-notification-tags", HelpText = "Semicolon-delineated list of GitHub tags (GitHub login or GitHub team) to notify in the case of non-batched subscription pull-request policy failure. Users must be publicly a member of the Microsoft org.", Default = "")] + public string PullRequestFailureNotificationTags { get; set; } + public override Operation GetOperation() { return new AddSubscriptionOperation(this); diff --git a/src/Microsoft.DotNet.Darc/src/DarcLib/Actions/Remote.cs b/src/Microsoft.DotNet.Darc/src/DarcLib/Actions/Remote.cs index c536eab3eb..d67b361744 100644 --- a/src/Microsoft.DotNet.Darc/src/DarcLib/Actions/Remote.cs +++ b/src/Microsoft.DotNet.Darc/src/DarcLib/Actions/Remote.cs @@ -237,7 +237,8 @@ public Task CreateSubscriptionAsync( string targetBranch, string updateFrequency, bool batchable, - List mergePolicies) + List mergePolicies, + string failureNotificationTags) { CheckForValidBarClient(); return _barClient.CreateSubscriptionAsync( @@ -247,7 +248,8 @@ public Task CreateSubscriptionAsync( targetBranch, updateFrequency, batchable, - mergePolicies); + mergePolicies, + failureNotificationTags); } /// diff --git a/src/Microsoft.DotNet.Darc/src/DarcLib/IBarClient.cs b/src/Microsoft.DotNet.Darc/src/DarcLib/IBarClient.cs index 22d5dfdc2e..9e5d34bc30 100644 --- a/src/Microsoft.DotNet.Darc/src/DarcLib/IBarClient.cs +++ b/src/Microsoft.DotNet.Darc/src/DarcLib/IBarClient.cs @@ -47,7 +47,8 @@ Task CreateSubscriptionAsync( string targetBranch, string updateFrequency, bool batchable, - List mergePolicies); + List mergePolicies, + string failureNotificationTags); /// /// Update an existing subscription diff --git a/src/Microsoft.DotNet.Darc/src/DarcLib/IRemote.cs b/src/Microsoft.DotNet.Darc/src/DarcLib/IRemote.cs index 6a253c2c3c..0448160894 100644 --- a/src/Microsoft.DotNet.Darc/src/DarcLib/IRemote.cs +++ b/src/Microsoft.DotNet.Darc/src/DarcLib/IRemote.cs @@ -149,7 +149,8 @@ Task CreateSubscriptionAsync( string targetBranch, string updateFrequency, bool batchable, - List mergePolicies); + List mergePolicies, + string failureNotificationTags); /// /// Update an existing subscription diff --git a/src/Microsoft.DotNet.Darc/src/DarcLib/MaestroApiBarClient.cs b/src/Microsoft.DotNet.Darc/src/DarcLib/MaestroApiBarClient.cs index cab1ccf927..9f195b3b89 100644 --- a/src/Microsoft.DotNet.Darc/src/DarcLib/MaestroApiBarClient.cs +++ b/src/Microsoft.DotNet.Darc/src/DarcLib/MaestroApiBarClient.cs @@ -235,7 +235,7 @@ private DependencyFlowEdge ToDependencyFlowEdge( /// /// Newly created subscription, if successful public Task CreateSubscriptionAsync(string channelName, string sourceRepo, string targetRepo, - string targetBranch, string updateFrequency, bool batchable, List mergePolicies) + string targetBranch, string updateFrequency, bool batchable, List mergePolicies, string failureNotificationTags) { var subscriptionData = new SubscriptionData( channelName: channelName, @@ -250,7 +250,8 @@ public Task CreateSubscriptionAsync(string channelName, string sou ignoreCase: true)) { MergePolicies = mergePolicies.ToImmutableList(), - }); + }, + failureNotificationTags); return _barClient.Subscriptions.CreateAsync(subscriptionData); }