Improving the Plan Process

[rawkode]

This is related to #128.

During the community call (#210) I said that "Plans should never fail".

I believe I was wrong.

If we want plans to show the user an accurate account of what will change on the system, we need to perform tasks that have the potential to error (Canonicalize a path, etc).

As the trait for plan is only to return Vec<Step> - we either return the steps we will action or an empty vector. This needs to change.

Instead, I'm proposing that plan returns something like Result<Vec<Step>> (This was actually proposed by @zbrox).

This makes a lot of sense as we can now emit an error here and tell the user that we failed to plan this action because X, Y, and Z.

However, this isn't enough. Again, referring to @128.

What if a previous action creates a file that is used further down the plan? Should that further action fail because it can't find that file? The answer is no, we should propagate that information throughout the plan. So I'd also like to propose an evolution of Step and Atom.

At the time of writing, these looks like:

pub struct Step {
    pub atom: Box<dyn Atom>,
    pub initializers: Vec<initializers::FlowControl>,
    pub finalizers: Vec<finalizers::FlowControl>,
}

pub trait Atom: std::fmt::Display {
    // Determine if this atom needs to run
    fn plan(&self) -> bool;

    // Apply new to old
    fn execute(&mut self) -> anyhow::Result<()>;

    // These methods allow for finalizers to query the outcome of the Atom.
    // We'll provide default implementations to allow Atoms to opt in to
    // the queries that make sense for them
    fn output_string(&self) -> String {
        String::from("")
    }

    fn error_message(&self) -> String {
        String::from("")
    }

    fn status_code(&self) -> i32 {
        0
    }
}

All of our Atoms actually map to side effects (FileCreate, FileDelete, FileChmod, etc). We don't make this information available to subsequent actions. As we "plan", we should build up these sideeffects and pass them down - providing a query interface for "Will this file be created prior to this step?"

[icepuma]

plan returns bool, not Step<Vec>?

[rawkode]

Only on Atoms, not Actions

[icepuma]

Gotcha. I saw the code sample and returned reading the text and was a bit mislead. But not your fault.

[icepuma]

Maybe I got this wrong:

1. Atoms are assembled into a list of steps
2. The list of steps is handed over to the apply function
3. The apply function filters for plannable atoms

I think we have to change the interface of the Atom trait:

pub trait Atom: std::fmt::Display {
    // Determine if this atom needs to run
    fn plan(&self, side_effects: Vec<SideEffect>) -> Result<Vec<SideEffect>>;
...
}

to make it fit into the current mechanism. The idea then would be to start with the first atom and an empty list of side_effects, go to the next atom, hand over the side_effects into the next atom and so on...

[icepuma]

I love the Result<Vec<Step>>. I can remember, that I was at this code lines and tempted to change it to return a Result to be able to use the try/? macro instead of unwrap and if let Some/Ok everything 💪