-
-
Notifications
You must be signed in to change notification settings - Fork 373
add support for nested task groups #3007
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
base: main
Are you sure you want to change the base?
Changes from all commits
6c2b758
82b12be
4f9bcb6
05170b5
3f7b3a8
7384a69
828df80
591a12c
369acc2
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 | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -79,7 +79,7 @@ private async System.Threading.Tasks.Task MigratePipelinesAsync() | |||||||
| } | ||||||||
| if (Options.MigrateTaskGroups) | ||||||||
| { | ||||||||
| taskGroupMappings = await CreateTaskGroupDefinitionsAsync(); | ||||||||
| taskGroupMappings = await CreateTaskGroupDefinitionsAsync(serviceConnectionMappings); | ||||||||
| } | ||||||||
| if (Options.MigrateBuildPipelines) | ||||||||
| { | ||||||||
|
|
@@ -200,8 +200,9 @@ private IEnumerable<TaskGroup> FilterOutExistingTaskGroups(IEnumerable<TaskGroup | |||||||
| /// </summary> | ||||||||
| /// <param name="filteredTaskGroups"></param> | ||||||||
| /// <param name="availableTasks"></param> | ||||||||
| /// <param name="taskGroupMappings"></param> | ||||||||
| /// <returns>List of filtered Definitions</returns> | ||||||||
| private IEnumerable<TaskGroup> FilterOutIncompatibleTaskGroups(IEnumerable<TaskGroup> filteredTaskGroups, IEnumerable<TaskDefinition> availableTasks) | ||||||||
| private IEnumerable<TaskGroup> FilterOutIncompatibleTaskGroupsWithMappings(IEnumerable<TaskGroup> filteredTaskGroups, IEnumerable<TaskDefinition> availableTasks, IEnumerable<Mapping> taskGroupMappings) | ||||||||
| { | ||||||||
| var objectsToMigrate = filteredTaskGroups.Where(g => | ||||||||
| { | ||||||||
|
|
@@ -212,6 +213,15 @@ private IEnumerable<TaskGroup> FilterOutIncompatibleTaskGroups(IEnumerable<TaskG | |||||||
| { | ||||||||
| return true; | ||||||||
| } | ||||||||
|
|
||||||||
| if (taskGroupMappings is not null) | ||||||||
| { | ||||||||
| if (taskGroupMappings.Any(m => m.SourceId == t.Task.Id)) | ||||||||
| { | ||||||||
| return true; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| missingTasksNames.Add(t.DisplayName); | ||||||||
| return false; | ||||||||
| }); | ||||||||
|
|
@@ -277,6 +287,11 @@ not null when typeof(DefinitionType) == typeof(ReleaseDefinition) => definitionN | |||||||
| .ToList(); | ||||||||
| } | ||||||||
|
|
||||||||
| private static bool IsMetaTask(string definitionType) | ||||||||
| { | ||||||||
| return string.Equals(definitionType, "metaTask", StringComparison.OrdinalIgnoreCase); | ||||||||
| } | ||||||||
|
|
||||||||
| private async Task<IEnumerable<Mapping>> CreateBuildPipelinesAsync(IEnumerable<Mapping> TaskGroupMapping = null, IEnumerable<Mapping> VariableGroupMapping = null, IEnumerable<Mapping> serviceConnectionMappings = null) | ||||||||
| { | ||||||||
| Log.LogInformation("Processing Build Pipelines.."); | ||||||||
|
|
@@ -311,7 +326,7 @@ private async Task<IEnumerable<Mapping>> CreateBuildPipelinesAsync(IEnumerable<M | |||||||
| { | ||||||||
| foreach (var step in phase.Steps) | ||||||||
| { | ||||||||
| if (step.Task.DefinitionType.ToLower() != "metaTask".ToLower()) | ||||||||
| if (!IsMetaTask(step.Task.DefinitionType)) | ||||||||
| { | ||||||||
| continue; | ||||||||
| } | ||||||||
|
|
@@ -527,7 +542,7 @@ private void UpdateTaskGroupId(ReleaseDefinition definitionToBeMigrated, IEnumer | |||||||
| { | ||||||||
| foreach (var WorkflowTask in deployPhase.WorkflowTasks) | ||||||||
| { | ||||||||
| if (WorkflowTask.DefinitionType != null && WorkflowTask.DefinitionType.ToLower() != "metaTask".ToLower()) | ||||||||
| if (!IsMetaTask(WorkflowTask.DefinitionType)) | ||||||||
| { | ||||||||
| continue; | ||||||||
| } | ||||||||
|
|
@@ -615,27 +630,107 @@ private async Task<IEnumerable<Mapping>> CreateServiceConnectionsAsync() | |||||||
| return mappings; | ||||||||
| } | ||||||||
|
|
||||||||
| private async Task<IEnumerable<Mapping>> CreateTaskGroupDefinitionsAsync() | ||||||||
| private async Task<IEnumerable<Mapping>> CreateTaskGroupDefinitionsAsync(IEnumerable<Mapping> serviceConnectionMappings) | ||||||||
| { | ||||||||
| Log.LogInformation($"Processing Taskgroups.."); | ||||||||
|
|
||||||||
| var sourceDefinitions = await Source.GetApiDefinitionsAsync<TaskGroup>(queryForDetails: false); | ||||||||
| var targetDefinitions = await Target.GetApiDefinitionsAsync<TaskGroup>(queryForDetails: false); | ||||||||
| var availableTasks = await Target.GetApiDefinitionsAsync<TaskDefinition>(queryForDetails: false); | ||||||||
| var filteredTaskGroups = FilterOutExistingTaskGroups(sourceDefinitions, targetDefinitions); | ||||||||
| filteredTaskGroups = FilterOutIncompatibleTaskGroups(filteredTaskGroups, availableTasks).ToList(); | ||||||||
| filteredTaskGroups = FilterOutIncompatibleTaskGroupsWithMappings(filteredTaskGroups, availableTasks, null).ToList(); | ||||||||
|
|
||||||||
| var existingMappings = FindExistingMappings(sourceDefinitions, targetDefinitions, new List<Mapping>()); | ||||||||
|
|
||||||||
| Log.LogInformation($"Phase 1 - Unnested Taskgroups"); | ||||||||
| var unnestedTaskGroups = filteredTaskGroups.Where(g => g.Tasks.All(t => !IsMetaTask(t.Task.DefinitionType))); | ||||||||
| existingMappings = await CreateTaskGroupsAsync(serviceConnectionMappings, targetDefinitions, availableTasks, unnestedTaskGroups, existingMappings); | ||||||||
|
|
||||||||
| Log.LogInformation($"Phase 2 - Nested Taskgroups"); | ||||||||
| var nestedTaskGroups = filteredTaskGroups.Where(g => g.Tasks.Any(t => IsMetaTask(t.Task.DefinitionType))).ToList(); | ||||||||
| var taskGroupsToMigrate = new List<TaskGroup>(); | ||||||||
|
|
||||||||
| do | ||||||||
| { | ||||||||
| // We need to process the nested taskgroups in a loop, because they can contain other nested taskgroups. | ||||||||
| taskGroupsToMigrate.Clear(); | ||||||||
| foreach (var taskGroup in nestedTaskGroups) | ||||||||
| { | ||||||||
| var nestedTaskGroup = taskGroup.Tasks.Where(t => IsMetaTask(t.Task.DefinitionType)).Select(t => t.Task).ToList(); | ||||||||
| if (nestedTaskGroup.All(t => existingMappings.Any(m => t.Id == m.SourceId))) | ||||||||
| { | ||||||||
| taskGroupsToMigrate.Add(taskGroup); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| nestedTaskGroups = nestedTaskGroups.Where(g => !taskGroupsToMigrate.Any(t => t.Id == g.Id)).ToList(); | ||||||||
| existingMappings = await CreateTaskGroupsAsync(serviceConnectionMappings, targetDefinitions, availableTasks, taskGroupsToMigrate, existingMappings); | ||||||||
|
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. I think we need to apped the new mappings here instead of overriding the whole list |
||||||||
| } while (nestedTaskGroups.Any() && taskGroupsToMigrate.Any()); | ||||||||
|
Comment on lines
+653
to
+668
|
||||||||
|
|
||||||||
| return existingMappings; | ||||||||
| } | ||||||||
|
|
||||||||
| private async Task<IEnumerable<Mapping>> CreateTaskGroupsAsync(IEnumerable<Mapping> serviceConnectionMappings, IEnumerable<TaskGroup> targetDefinitions, IEnumerable<TaskDefinition> availableTasks, IEnumerable<TaskGroup> filteredTaskGroups, IEnumerable<Mapping> existingMappings) | ||||||||
| { | ||||||||
| filteredTaskGroups = FilterOutIncompatibleTaskGroupsWithMappings(filteredTaskGroups, availableTasks, existingMappings).ToList(); | ||||||||
|
|
||||||||
| var rootSourceDefinitions = SortDefinitionsByVersion(filteredTaskGroups).First(); | ||||||||
| var updatedSourceDefinitions = SortDefinitionsByVersion(filteredTaskGroups).Last(); | ||||||||
|
|
||||||||
| foreach (var definitionToBeMigrated in rootSourceDefinitions) | ||||||||
| { | ||||||||
| if (serviceConnectionMappings is not null) | ||||||||
| { | ||||||||
| foreach (var task in definitionToBeMigrated.Tasks) | ||||||||
| { | ||||||||
| var newInputs = new Dictionary<string, object>(); | ||||||||
| foreach (var input in (IDictionary<String, Object>)task.Inputs) | ||||||||
| { | ||||||||
| var mapping = serviceConnectionMappings.FirstOrDefault(d => d.SourceId == input.Value.ToString()); | ||||||||
|
||||||||
| var mapping = serviceConnectionMappings.FirstOrDefault(d => d.SourceId == input.Value.ToString()); | |
| var mapping = serviceConnectionMappings.FirstOrDefault(d => d.SourceId == input.Value?.ToString()); |
Copilot
AI
Oct 17, 2025
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.
This code removes and re-adds dictionary entries unnecessarily. Instead of removing and adding, directly update the value: ((IDictionary<String, Object>)task.Inputs)[input.Key] = input.Value;
| ((IDictionary<String, Object>)task.Inputs).Remove(input.Key); | |
| ((IDictionary<String, Object>)task.Inputs).Add(input.Key, input.Value); | |
| ((IDictionary<String, Object>)task.Inputs)[input.Key] = input.Value; |
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.
Why do we need this? As far as i see, there are no changes to this list. Therefore we can just reuse it in the method calling this method
Copilot
AI
Oct 17, 2025
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.
Duplicate mappings may be added to the list. The existingMappings are being added after potentially creating new mappings from the same source definitions in line 732. This could result in duplicate entries if the same mapping exists in both collections. Consider using mappings = mappings.Union(existingMappings).ToList() or check for duplicates before adding.
| mappings.AddRange(existingMappings); | |
| mappings = mappings.Union(existingMappings).ToList(); |
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.
Maybe instead of inititalizing a new list here, amke the parameter optional and initialize it in the method itself if not set