Model templates #92
Model templates #92
Conversation
cmd/fablab/subcmd/cat.go
Outdated
| } | ||
|
|
||
| var getFilesCmd = &cobra.Command{ | ||
| Use: "get-files hostSpec localPath remoteFiles", |
There was a problem hiding this comment.
get-files <hostSpec> <localPath> <remoteFiles> ... very minor nitpick.
There was a problem hiding this comment.
Also, can we add a get command, which contains the files command... rather than a hyphenated get-files?
kernel/model/bootstrap.go
Outdated
| @@ -88,6 +88,12 @@ func bootstrapModel() (*Model, error) { | |||
| } | |||
| } | |||
|
|
|||
| for _, factory := range m.ModelFactories { | |||
| if err := factory.Build(m); err != nil { | |||
| return nil, fmt.Errorf("error executing factory [%s] (%w)", reflect.TypeOf(factory), err) | |||
There was a problem hiding this comment.
been trying to standardize on errors.Wrap for this sort of thing.
return nil, errors.Wrapf(err, "error executing factory [%s]", reflect.TypeOf(factory))
There was a problem hiding this comment.
Good to know, I hadn't learned about errors.Wrap(f) yet. I've started converting things over, but there's a lot there, so I'll convert more opportunistically as I come across them.
| Regions Regions | ||
|
|
||
| Regions Regions | ||
| ModelFactories []Factory // Factories that change the model structure, eg: add/remove hosts |
There was a problem hiding this comment.
Why would this be different than a Factory? Why add another Factory concept?
There was a problem hiding this comment.
It's to separate Factories which mutate model structure vs ones that don't. The structure manipulation has to be done before we bind the label and environment, otherwise things like IP addresses don't end up in the right spot, b/c their intended targets aren't there yet. Other Factories get done after the label/binding are applied, and it didn't feel right to move them all in front of label/env binding.
Let me know if that makes sense, if you've got ideas on how that could be solved differently.
There was a problem hiding this comment.
I think by their nature factories need to be inserted into the model in the correct order. What you're doing with "model" factories is no different.. they just happen first. Not sure that it's a useful distinction. It's more powerful to just have "factories" and put them in the correct order.
There was a problem hiding this comment.
Are you arguing there should be just one set of factories and that they should all be invoked before the label and environment is bound? I suspect that will break other factories which rely on variables to be set.
There was a problem hiding this comment.
Why can't all of the factories run after the label and environment are bound? The label and environment binding just feed static data to the factories and the model?
There was a problem hiding this comment.
Maybe label/environment binding gets moved into a Factory and then the ordering can be controlled however we want per-model? (If we really do have factories that we want to run before label/environment binding)
There was a problem hiding this comment.
Moving label/env binding into a factory would work. The templating stuff really does need to run before binding, otherwise the binding doesn't have anything to apply values to.
There was a problem hiding this comment.
We should probably loop this into our conversation tomorrow.
| return result | ||
| } | ||
|
|
||
| func (region *Region) Templatize(templater *Templater) { |
There was a problem hiding this comment.
I think my biggest issue with "templatization" is the name. If all we're doing is calling CloneXXX methods from a Factory... why not just call it that? Then, doesn't the model just contain a single, or the smallest version of a footprint, and we're just cloning that to extend the model to a larger configuration?
I just don't think the model is a "template" in that example. It's a fully-fledged model.
There was a problem hiding this comment.
We can change the name, but there is more to it than just cloning. Taking the edge bit for an example, here's the client host/component definition:
"client{{ .Index }}": {
Scope: model.Scope{Tags: model.Tags{"^client", "^sdk-app"}},
Components: model.Components{
"client{{ .Host.Index }}": {
BinaryName: "ziti-fabric-test",
PublicIdentity: "client{{ .Host.Index }}",
ConfigSrc: "loop/edge-perf.loop3.yml",
ConfigName: "edge-perf-{{ .Host.Index }}.loop3.yml",
},
},
},
So we're not just copying it, we're using it as a base, and injecting environment into the id, public identity and config name. Even if we only had one client, it wouldn't look like the code above, it would be the version after templating has been applied.
Does that justify the templating nomenclature, or does it still feel like it should be called something else?
There was a problem hiding this comment.
I need to spend more time thinking about this. I don't think bringing the golang template engine into the mix makes this more streamlined... I feel like it makes it more convoluted. Using the model to express things that are "templates" feels like a corruption of the model.
plorenz
force-pushed
the
model-templates
branch
2 times, most recently
from
936f054
to
021d1f7
Compare
|
Replaced by refactored model scaling PR #113 |
No description provided.