Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ModularPipelines/Engine/IModuleConditionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ namespace ModularPipelines.Engine;

internal interface IModuleConditionHandler
{
Task<(bool ShouldIgnore, SkipDecision? SkipDecision)> ShouldIgnore(IModule module);
Task<(bool ShouldIgnore, SkipDecision? SkipDecision)> ShouldIgnore(IModule module, CancellationToken cancellationToken = default);
}
43 changes: 21 additions & 22 deletions src/ModularPipelines/Engine/ModuleConditionHandler.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Reflection;
using EnumerableAsyncProcessor.Extensions;
using Microsoft.Extensions.Options;
using ModularPipelines.Attributes;
using ModularPipelines.Context;
Expand All @@ -20,7 +19,7 @@ public ModuleConditionHandler(IOptions<PipelineOptions> pipelineOptions, IPipeli
_pipelineContextProvider = pipelineContextProvider;
}

public async Task<(bool ShouldIgnore, SkipDecision? SkipDecision)> ShouldIgnore(IModule module)
public async Task<(bool ShouldIgnore, SkipDecision? SkipDecision)> ShouldIgnore(IModule module, CancellationToken cancellationToken = default)
{
var moduleType = module.GetType();

Expand All @@ -34,7 +33,7 @@ public ModuleConditionHandler(IOptions<PipelineOptions> pipelineOptions, IPipeli
return (true, SkipDecision.Skip("The module was not in a runnable category"));
}

var conditionResult = await IsRunnableCondition(moduleType).ConfigureAwait(false);
var conditionResult = await IsRunnableCondition(moduleType, cancellationToken).ConfigureAwait(false);
if (!conditionResult.IsRunnable)
{
return (true, conditionResult.SkipDecision);
Expand Down Expand Up @@ -71,43 +70,43 @@ private bool IsIgnoreCategory(Type moduleType)
return category != null && ignoreCategories.Contains(category.Category);
}

private async Task<(bool IsRunnable, SkipDecision? SkipDecision)> IsRunnableCondition(Type moduleType)
private async Task<(bool IsRunnable, SkipDecision? SkipDecision)> IsRunnableCondition(Type moduleType, CancellationToken cancellationToken)
{
var mandatoryRunConditionAttributes = moduleType.GetCustomAttributes<MandatoryRunConditionAttribute>(true).ToList();
var runConditionAttributes = moduleType.GetCustomAttributes<RunConditionAttribute>(true).Except(mandatoryRunConditionAttributes).ToList();

// Get a context for condition evaluation
var pipelineContext = _pipelineContextProvider.GetModuleContext();

var mandatoryConditionResults = await mandatoryRunConditionAttributes.ToAsyncProcessorBuilder()
.SelectAsync(async runConditionAttribute => new RunnableConditionMet(await runConditionAttribute.Condition(pipelineContext).ConfigureAwait(false), runConditionAttribute))
.ProcessInParallel();

var mandatoryCondition = mandatoryConditionResults.FirstOrDefault(result => !result.ConditionMet);

if (mandatoryCondition != null)
// Evaluate mandatory conditions sequentially with short-circuit on first failure
foreach (var attr in mandatoryRunConditionAttributes)
{
return (false, SkipDecision.Skip($"A condition to run this module has not been met - {mandatoryCondition.RunConditionAttribute.GetType().Name}"));
cancellationToken.ThrowIfCancellationRequested();
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cancellation token is being checked between evaluating conditions, but it's not being passed to the attr.Condition(pipelineContext) call on line 86. This means the actual condition evaluation cannot be cancelled, which limits the effectiveness of the cancellation support added in this PR. Consider updating the RunConditionAttribute.Condition method signature to accept a CancellationToken parameter.

Copilot uses AI. Check for mistakes.

var conditionMet = await attr.Condition(pipelineContext).ConfigureAwait(false);
if (!conditionMet)
{
return (false, SkipDecision.Skip($"A condition to run this module has not been met - {attr.GetType().Name}"));
}
}

if (!runConditionAttributes.Any())
{
return (true, null);
}

var conditionResults = await runConditionAttributes.ToAsyncProcessorBuilder()
.SelectAsync(async runConditionAttribute => new RunnableConditionMet(await runConditionAttribute.Condition(pipelineContext).ConfigureAwait(false), runConditionAttribute))
.ProcessInParallel();

var runnableCondition = conditionResults.FirstOrDefault(result => result.ConditionMet);

if (runnableCondition != null)
// Evaluate non-mandatory conditions sequentially with short-circuit on first success
foreach (var attr in runConditionAttributes)
{
return (true, null);
cancellationToken.ThrowIfCancellationRequested();
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cancellation token is being checked between evaluating conditions, but it's not being passed to the attr.Condition(pipelineContext) call on line 103. This means the actual condition evaluation cannot be cancelled, which limits the effectiveness of the cancellation support added in this PR. Consider updating the RunConditionAttribute.Condition method signature to accept a CancellationToken parameter.

Copilot uses AI. Check for mistakes.

var conditionMet = await attr.Condition(pipelineContext).ConfigureAwait(false);
if (conditionMet)
{
return (true, null);
}
}

return (false, SkipDecision.Skip($"No run conditions were met: {string.Join(", ", runConditionAttributes.Select(x => x.GetType().Name.Replace("Attribute", string.Empty, StringComparison.OrdinalIgnoreCase)))}"));
}

private record RunnableConditionMet(bool ConditionMet, RunConditionAttribute RunConditionAttribute);
}
Loading