create a metadata envelope for nodes

[BrianHicks]

These changes are enable future expansion in the following:

• better errors (keep metadata for location and include it)
• letting preparers know their names

Points of interest in this PR:

• add a metadata wrapper for future expansion (graph/node/node.go)
• graph.Get return value changes from interface{} to (meta *node.Node, ok bool). This is basically removing Get in favor of MaybeGet and
  renaming/reworking everything to use the new return value.
• graph.GetParent return value changes from interface{} to (meta *node.Node, ok bool) in the same way.
• walking functions change from interface{} to *node.Node

This PR changes a whole lot of stuff, but it's all in service of the changes
above. This might cause some integration headaches but should improve failure
detection, since call sites now have to explicitly handle missing nodes (they
didn't before, oops)

[rebeccaskinner]

Not requesting changes because I think they are more open design questions about if/when we should do some things.

[rebeccaskinner]

Since we are using the ID inside of the pipeline, does it make sense to actually go ahead and pass the entire node into the pipeline instead of extracting it, then pull the ID from the node rather than putting it into the pipeline?

[BrianHicks]

What would that look like? I'm having a hard time understanding what you're saying here.

[rebeccaskinner]

inside of apply/pipeline we have:

type pipelineGen struct {
    Graph          *graph.Graph
    ID             string
    RenderingPlant *render.Factory
}

and

func Pipeline(g *graph.Graph, id string, factory *render.Factory) executor.Pipeline {
    gen := &pipelineGen{Graph: g, RenderingPlant: factory, ID: id}

since the ID is now going to be part of the element we could remove the ID from pipelineGen and the call to Pipeline. It's not necessarily hurting anything to have it as it is, but it does get us down to a single source-of-truth for the ID.

[BrianHicks]

hmm, that seems like it would be a good future addition. One of the optimizations we've been talking about, yeah?

[rebeccaskinner]

yeah that's fair 👍

[rebeccaskinner]

As with apply- does it make sense to pass the entire node in here and handle it correctly inside the pipeline?

[rebeccaskinner]

I've noticed that we have a general pattern of at each phase in the processing pipeline (load,render,plan,and apply) we create a new *Node, with the ID and value. That works for now but it does require that we have knowlege that the id and value are the only relevant fields in the node, and that nothing earlier in the pipeline has added useful metadata.

Would it make sense to have a func (n *Node) SetValue(newValue interface{}) { that would set the value, or else a func (n *Node) Transform(func (interface{}) (interface{}, error) ) (*Node, error) that transforms the value inside of the node, so that later phases of the processing pipeline can update the inner value without losing accumulated metadata information?

[BrianHicks]

yeah, we need to not lose accumulated metadata… SetValue works but it means we're mutating the nodes. We could copy them, of course, but that essentially means throwing away an entire graph at every stage. I'm not sure what the right solution is. Since our processing only goes forward and never forks, mutating might make sense. What do you think?

[rebeccaskinner]

I prefer the idea of copying because it doesn't feel like the size of the graph is going to be large enough that it's going to introduce enormous overhead; we already replace the node at each stage so we'd only be introducing the addition of copying the metadata which doesn't seem like it should be all that much extra?

Also in the absence of a compelling reason to mutate I'd say that we're better to prefer copying just to avoid bugs related to parallel execution- I don't really have any specific case in mind where that would happen but it does just seem like the sort of thing.

[rebeccaskinner]

LGTM on green