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

Pipeline execution order #2410

Open
KathleenDollard opened this issue Apr 29, 2024 · 1 comment
Open

Pipeline execution order #2410

KathleenDollard opened this issue Apr 29, 2024 · 1 comment
Labels
Design Powderhouse Work to isolate parser and features

Comments

@KathleenDollard
Copy link
Contributor

We have taken or considered many approaches to pipeline ordering. What we have observed:

  • The order is essential for correct operation. You can't validate before you display help.
  • The order is nuanced and very difficult to predict casually.
  • Subsystems (called middleware in past designs) fall into distinct categories.
  • Powderhouse leans even more heavily into subsystems as anything extensible is probably a subsystem.

The groupings are these:

  • Early termination
    • Help
    • Version
    • Some directives like diagramming and completions
  • Invocation preprocessing
    • Validation
    • Error reporting
  • Invocation
    • Invocation
  • Teardown
    • Exception reporting (if we retain it)
  • Independent (these serve other subsystems)
    • Values (which includes default values)

Relavent design decisions

  • Any subsystem can be replaced by a subsystem that derives from it.
  • Subsystems that we do not provide are welcome. We have several ideas ;)
  • We anticipate rare but important cases where multiple subsystems of the same type are used - such as help systems for different parts of a complex CLI (dotnet new in the .NET CLI). We anticipate these CLI authors will create an aggregating subsystem and route.

Proposal

Create pipeline groups with great names for:

  • EarlyTerminating
  • BeforeInvocation
  • Invocation
  • AfterInvocation
  • NonExecuting

Hey, are those great names?

New subsystems would be placed in a group:

public void Add(CliSubsystem subsystem, PipelineGroup pipelineGroup, bool runFirst = false);

We think it will rarely matter whether something runs first or last within its group. It would only matter when the new subsystem alters the behavior of another subsystem. To allow further customization within the group, we can commit to retaining the order things were added at eitehr the first or last position in the group. There will be only obtuse (*) ability to change the order of standard subsystems.

(*) We can't keep someone from removing it at its normal position and placing it elsewhere. That seems pathological.

Other approaches considered

  • A free form list (we tried this a while back, it was a huge source of errors).
  • Transposing user entered subsystems/middleware (I think this is the current approach on main. It's complicated.)
  • Having only standard slots, and if you want to add something, you pick the right one, create a new pipeline, and derive from methods like ExecuteHelp (weird and complicated).
  • Numbering with gaps in between for you to add your subsystem in a specific place, with no ability to alter order of standard subsystems (to be honest, we didn't consider this for very long.)
@KathleenDollard KathleenDollard added Design Powderhouse Work to isolate parser and features labels Apr 29, 2024
@KathleenDollard
Copy link
Contributor Author

KathleenDollard commented May 3, 2024

Alternate names and including an API:

public enum CliPipelinePhases
{
   // I am actually not sure we need this, as ValueSubsystem needs to act like it is executing to get its data
   NonExecuting = 0,

   /// <summary>Operations that do not require data prep and that generally supersede invocation.</summary>
   EarlyTermination, // NonData?

   /// <summary>Operations that work with data prior to invocation, such as validation. These can terminate on error.</summary>  
   DataPreparation, // ValuePreperation?, BeforeInvocation?

   /// <summary>Invoking operations</summary
   Invocation,

   /// <summary>Error reporting, ExitCode transposition, possibly exception handling, cleanup</summary
   Finishing
}

public class Pipeline
{
   //...
   public void AddSubsystem(CliSubSystem subsystem, CliPipelinePhase phase, bool atStart = false) {...}
   //...
}

The pipeline would also continue to have properties for the expected subsystems, such as Help that could be used to replace the default, so this would not be the common case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Powderhouse Work to isolate parser and features
Projects
None yet
Development

No branches or pull requests

1 participant