Skip to content
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

API Review Feedback #1520

Merged
merged 7 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public CircuitBreakerPredicateArguments(ResilienceContext context, Outcome<TResu
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome of user-callback.
martincostello marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }
}
4 changes: 2 additions & 2 deletions src/Polly.Core/CircuitBreaker/OnCircuitClosedArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ public OnCircuitClosedArguments(ResilienceContext context, Outcome<TResult> outc
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome that caused the circuit breaker to be closed.
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }

Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Core/CircuitBreaker/OnCircuitOpenedArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ public OnCircuitOpenedArguments(ResilienceContext context, Outcome<TResult> outc
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome that caused the circuit to open.
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }

Expand Down
34 changes: 34 additions & 0 deletions src/Polly.Core/Fallback/FallbackActionArguments.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
namespace Polly.Fallback;

#pragma warning disable CA1815 // Override equals and operator equals on value types

/// <summary>
/// Arguments used by <see cref="FallbackStrategyOptions{TResult}.FallbackAction"/>.
/// </summary>
/// <typeparam name="TResult">The type of result.</typeparam>
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly struct FallbackActionArguments<TResult> : IOutcomeArguments<TResult>
{
/// <summary>
/// Initializes a new instance of the <see cref="FallbackActionArguments{TResult}"/> struct.
/// </summary>
/// <param name="outcome">The context in which the resilience operation or event occurred.</param>
/// <param name="context">The outcome of the resilience operation or event.</param>
public FallbackActionArguments(ResilienceContext context, Outcome<TResult> outcome)
{
Context = context;
Outcome = outcome;
}

/// <summary>
/// Gets the outcome that should be handled by the fallback.
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }
}
6 changes: 3 additions & 3 deletions src/Polly.Core/Fallback/FallbackHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ namespace Polly.Fallback;

internal sealed record class FallbackHandler<T>(
Func<FallbackPredicateArguments<T>, ValueTask<bool>> ShouldHandle,
Func<FallbackPredicateArguments<T>, ValueTask<Outcome<T>>> ActionGenerator)
Func<FallbackActionArguments<T>, ValueTask<Outcome<T>>> ActionGenerator)
{
public async ValueTask<Outcome<TResult>> GetFallbackOutcomeAsync<TResult>(FallbackPredicateArguments<T> args)
public async ValueTask<Outcome<TResult>> GetFallbackOutcomeAsync<TResult>(FallbackActionArguments<T> args)
{
var copiedArgs = new FallbackPredicateArguments<T>(
var copiedArgs = new FallbackActionArguments<T>(
args.Context,
args.Outcome.AsOutcome<T>());

Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Core/Fallback/FallbackPredicateArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public FallbackPredicateArguments(ResilienceContext context, Outcome<TResult> ou
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome of user-callback.
martincostello marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }
}
2 changes: 1 addition & 1 deletion src/Polly.Core/Fallback/FallbackResilienceStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ protected internal override async ValueTask<Outcome<T>> ExecuteCore<TState>(Func

try
{
return await _handler.GetFallbackOutcomeAsync<T>(handleFallbackArgs).ConfigureAwait(context.ContinueOnCapturedContext);
return await _handler.GetFallbackOutcomeAsync<T>(new FallbackActionArguments<T>(context, outcome)).ConfigureAwait(context.ContinueOnCapturedContext);
}
catch (Exception e)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Polly.Core/Fallback/FallbackStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class FallbackStrategyOptions<TResult> : ResilienceStrategyOptions
/// The default value is <see langword="null"/>. This property is required.
/// </value>
[Required]
public Func<FallbackPredicateArguments<TResult>, ValueTask<Outcome<TResult>>>? FallbackAction { get; set; }
public Func<FallbackActionArguments<TResult>, ValueTask<Outcome<TResult>>>? FallbackAction { get; set; }

/// <summary>
/// Gets or sets event delegate that is raised when fallback happens.
Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Core/Fallback/OnFallbackArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public OnFallbackArguments(ResilienceContext context, Outcome<TResult> outcome)
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome that caused the fallback to be executed.
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }
}
4 changes: 2 additions & 2 deletions src/Polly.Core/Hedging/Controller/HedgingHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ internal sealed record class HedgingHandler<T>(
args.PrimaryContext,
args.ActionContext,
args.AttemptNumber,
(Func<ResilienceContext, ValueTask<Outcome<T>>>)(object)args.Callback);
args.Callback);

return (Func<ValueTask<Outcome<T>>>?)(object)ActionGenerator(copiedArgs)!;
return ActionGenerator(copiedArgs);
}

return CreateNonGenericAction(args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ namespace Polly.Hedging;
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly struct HedgingDelayArguments
public readonly struct HedgingDelayGeneratorArguments
{
/// <summary>
/// Initializes a new instance of the <see cref="HedgingDelayArguments"/> struct.
/// Initializes a new instance of the <see cref="HedgingDelayGeneratorArguments"/> struct.
/// </summary>
/// <param name="context">The context associated with the execution of a user-provided callback.</param>
/// <param name="attemptNumber">The zero-based hedging attempt number.</param>
public HedgingDelayArguments(ResilienceContext context, int attemptNumber)
public HedgingDelayGeneratorArguments(ResilienceContext context, int attemptNumber)
{
Context = context;
AttemptNumber = attemptNumber;
Expand Down
4 changes: 2 additions & 2 deletions src/Polly.Core/Hedging/HedgingPredicateArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public HedgingPredicateArguments(ResilienceContext context, Outcome<TResult> out
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome of user-callback.
martincostello marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public Outcome<TResult> Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }
}
12 changes: 6 additions & 6 deletions src/Polly.Core/Hedging/HedgingResilienceStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public HedgingResilienceStrategy(
int maxHedgedAttempts,
HedgingHandler<T> hedgingHandler,
Func<OnHedgingArguments<T>, ValueTask>? onHedging,
Func<HedgingDelayArguments, ValueTask<TimeSpan>>? hedgingDelayGenerator,
Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? hedgingDelayGenerator,
TimeProvider timeProvider,
ResilienceStrategyTelemetry telemetry)
{
Expand All @@ -34,7 +34,7 @@ public HedgingResilienceStrategy(

public int MaxHedgedAttempts { get; }

public Func<HedgingDelayArguments, ValueTask<TimeSpan>>? HedgingDelayGenerator { get; }
public Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? HedgingDelayGenerator { get; }

public HedgingHandler<T> HedgingHandler { get; }

Expand Down Expand Up @@ -95,7 +95,7 @@ private async ValueTask<Outcome<T>> ExecuteCoreAsync<TState>(
// If completedHedgedTask is null it indicates that we still do not have any finished hedged task within the hedging delay.
// We will create additional hedged task in the next iteration.
await HandleOnHedgingAsync(
new OnHedgingArguments<T>(context, Outcome.FromResult<T>(default), attempt, hasOutcome: false, duration: delay)).ConfigureAwait(context.ContinueOnCapturedContext);
new OnHedgingArguments<T>(context, null, attempt, duration: delay)).ConfigureAwait(context.ContinueOnCapturedContext);
continue;
}

Expand All @@ -109,13 +109,13 @@ await HandleOnHedgingAsync(

var executionTime = _timeProvider.GetElapsedTime(start);
await HandleOnHedgingAsync(
new OnHedgingArguments<T>(context, outcome, attempt, hasOutcome: true, executionTime)).ConfigureAwait(context.ContinueOnCapturedContext);
new OnHedgingArguments<T>(context, outcome, attempt, executionTime)).ConfigureAwait(context.ContinueOnCapturedContext);
}
}

private async ValueTask HandleOnHedgingAsync(OnHedgingArguments<T> args)
{
_telemetry.Report<OnHedgingArguments<T>, T>(new(ResilienceEventSeverity.Warning, HedgingConstants.OnHedgingEventName), args);
_telemetry.Report<OnHedgingArguments<T>, T>(new(ResilienceEventSeverity.Warning, HedgingConstants.OnHedgingEventName), args.Context, default, args);

if (OnHedging is not null)
{
Expand All @@ -133,6 +133,6 @@ internal ValueTask<TimeSpan> GetHedgingDelayAsync(ResilienceContext context, int
return new ValueTask<TimeSpan>(HedgingDelay);
}

return HedgingDelayGenerator(new HedgingDelayArguments(context, attempt));
return HedgingDelayGenerator(new HedgingDelayGeneratorArguments(context, attempt));
}
}
2 changes: 1 addition & 1 deletion src/Polly.Core/Hedging/HedgingStrategyOptions.TResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public class HedgingStrategyOptions<TResult> : ResilienceStrategyOptions
/// <value>
/// The default value is <see langword="null"/>.
/// </value>
public Func<HedgingDelayArguments, ValueTask<TimeSpan>>? HedgingDelayGenerator { get; set; }
public Func<HedgingDelayGeneratorArguments, ValueTask<TimeSpan>>? HedgingDelayGenerator { get; set; }

/// <summary>
/// Gets or sets the event that is raised when a hedging is performed.
Expand Down
21 changes: 6 additions & 15 deletions src/Polly.Core/Hedging/OnHedgingArguments.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,31 @@ namespace Polly.Hedging;
/// <remarks>
/// Always use the constructor when creating this struct, otherwise we do not guarantee binary compatibility.
/// </remarks>
public readonly struct OnHedgingArguments<TResult> : IOutcomeArguments<TResult>
public readonly struct OnHedgingArguments<TResult>
{
/// <summary>
/// Initializes a new instance of the <see cref="OnHedgingArguments{TResult}"/> struct.
/// </summary>
/// <param name="outcome">The context in which the resilience operation or event occurred.</param>
/// <param name="context">The outcome of the resilience operation or event.</param>
/// <param name="attemptNumber">The zero-based hedging attempt number.</param>
/// <param name="hasOutcome">Indicates whether outcome is available.</param>
/// <param name="duration">The execution duration of hedging attempt or the hedging delay in case the attempt was not finished in time.</param>
public OnHedgingArguments(ResilienceContext context, Outcome<TResult> outcome, int attemptNumber, bool hasOutcome, TimeSpan duration)
public OnHedgingArguments(ResilienceContext context, Outcome<TResult>? outcome, int attemptNumber, TimeSpan duration)
{
Context = context;
Outcome = outcome;
AttemptNumber = attemptNumber;
HasOutcome = hasOutcome;
Duration = duration;
}

/// <summary>
/// Gets the outcome of the resilience operation or event.
/// Gets the outcome that needs to be hedged, if any.
/// </summary>
public Outcome<TResult> Outcome { get; }
/// <remarks>If this property is <see langword="null"/>, it's an indication that user-callback or hedged operation did not complete within the hedging delay.</remarks>
public Outcome<TResult>? Outcome { get; }

/// <summary>
/// Gets the context in which the resilience operation or event occurred.
/// Gets the context of this event.
/// </summary>
public ResilienceContext Context { get; }

Expand All @@ -43,14 +42,6 @@ public OnHedgingArguments(ResilienceContext context, Outcome<TResult> outcome, i
/// </summary>
public int AttemptNumber { get; }

/// <summary>
/// Gets a value indicating whether the outcome is available before loading the next hedged task.
/// </summary>
/// <remarks>
/// No outcome indicates that the previous action did not finish within the hedging delay.
/// </remarks>
public bool HasOutcome { get; }

/// <summary>
/// Gets the execution duration of hedging attempt or the hedging delay in case the attempt was not finished in time.
/// </summary>
Expand Down
Loading