ManageIQ / manageiq

ManageIQ Open-Source Management Platform
https://manageiq.org
Apache License 2.0
1.34k stars 899 forks source link

Reorganizing the ServiceTemplate model and its children #20603

Open skateman opened 3 years ago

skateman commented 3 years ago

I started working on the catalog item forms and I am bumping into architectural obstacles. There's only a partial (and a little bit weird) STI implemented around the ServiceTemplate model. Right now we're accessing the list of the possible types via ExtManagementSystem.supported_types_for_catalog and then ExtManagementSystem.catalog_types which for each catalog returns a hash with one or zero elements indexed by a string that is stored in per record as prov_type. This string is everything but deterministic, we are doing a lot of map-reduce magic to work with this.

As I was going through the models, I found that the following classes are inherited from the ServiceTemplate:

Also these below, but I guess I don't have to deal with them as they seem like v2v models * `ServiceTemplateTransformationPlan` * `ServiceTemplateTransformationPlanRequest` * `ServiceTemplateTransformationPlanTask`

There are some inconsistencies in the mapping of prov_type as well, for example when it's set to generic it doesn't map to ServiceTemplateGeneric but all the others except ServiceTemplateOrchestration are inherited from it. I really tried to work with these without changes, but I realized that I could probably produce cleaner code if I cleaned this up first. So here is my proposal:

I really tried to avoid mentioning DDF in the steps above :laughing: because there are just a few actual DDF fields that should be actually stored in the models, most of the schema is already described in MiqDialog records and we just need a way to convert those to the right format (which can be just done on the frontend). The goal here is to have something similar to what we did to the provider forms.

@agrare @Fryguy wdyt?

Fryguy commented 3 years ago

Overall, I agree that this needs a rework. STI seems like a decent approach to me. However, it may not be necessary, as I think the architectural direction was to go towards ServiceTemplateGeneric and drop the rest. I might be wrong, though, so we should get together with @lfu @tinaafitz (and probably @gmcculloug) to hash it out.

skateman commented 3 years ago

If we don't go with the STI approach, we have to add some additional complexity that sorts out the provider-specific fields for each subtype here :confused: even though most of the provider-specific stuff is through the provisioning dialogs specified by the YAML files, there are a few fields that are not in the YAML different for each provider :confused: