Generator redesign read meta for resource name to class struct (DCNE-345) #1350
Conversation
akinross
requested review from
anvitha-jain,
gmicol,
lhercot,
sajagana,
samiib and
shrsr
as code owners
akinross
changed the title
akinross
force-pushed
the
generator_redesign_read_meta_to_class_struct
branch
from
aba6ba9 to
9012df2
Compare
gen/utils/data/class.go
Outdated
| func (c *Class) setResourceName() error { | ||
| genLogger.Debug(fmt.Sprintf("Setting resource name for class '%s'.", c.ClassName)) | ||
|
|
||
| // TODO: add logic to override the resource name from an definition file. |
There was a problem hiding this comment.
Not important but..
->
// TODO: add logic to override the resource name from a definition file.
There was a problem hiding this comment.
assuming the change was an to a, changed this now
gen/utils/data/class.go
Outdated
| if label, ok := c.MetaFileContent["label"]; ok && label != "" { | ||
| c.ResourceName = utils.Underscore(label.(string)) | ||
| } else { | ||
| return fmt.Errorf("failed to set resource name for class '%s': label not found", c.ClassName) | ||
| } |
There was a problem hiding this comment.
This also does not account for relational classes right where we need to add relation_ right?
There was a problem hiding this comment.
ok will include logic for this
There was a problem hiding this comment.
included logic for this, added some todo notes for overwrites that we would likely require
gen/utils/data/class.go
Outdated
| // For relational class nested attributes the name is changed to 'to_resource_name' when the resource name includes 'from_resource_name'. | ||
| // ex. 'relation_from_bridge_domain_to_netflow_monitor_policy' would translate to 'relation_to_netflow_monitor_policy'. | ||
| m := regexp.MustCompile("from_(.*)_to_") | ||
| if m.MatchString(c.ResourceNameNested) { | ||
| c.ResourceNameNested = m.ReplaceAllString(c.ResourceNameNested, "to_") | ||
| } |
There was a problem hiding this comment.
No sure why this is needed as we could just do it before as a classic set or make this part of the resource name function itself. Not sure we want this as a separate function.
There was a problem hiding this comment.
yes did not account for constructing the relation_ from_ and to_ parts, will change the logic with the comment above.
| for _, test := range tests { | ||
| genLogger.Info(fmt.Sprintf("Executing: %s' with input '%s' and expected output '%s'", t.Name(), test["input"], test["expected"])) | ||
| result, err := Plural(test["input"]) | ||
| assert.NoError(t, err, fmt.Sprintf("Expected no error, but got '%s'", err)) | ||
| assert.Equal(t, test["expected"], result, fmt.Sprintf("Expected '%s', but got '%s'", test["expected"], result)) | ||
| } |
There was a problem hiding this comment.
This seems to be repeated in some fashion quite a bit. Should this be a function that accept a series of attribute including the tested function?
There was a problem hiding this comment.
will have a look at creating functions for all the repeating tests
There was a problem hiding this comment.
had a look and think it will if we want to standardise this we should have a follow up discussion, for now created isolated test cases
akinross
force-pushed
the
generator_redesign_read_meta_to_class_struct
branch
from
66d4405 to
71c61e0
Compare
…source name setting for relationships
akinross
force-pushed
the
generator_redesign_read_meta_to_class_struct
branch
from
71c61e0 to
6541502
Compare
fixes DCNE-345