-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add PathAndStruct helper for generated non-leaf PathStruct types. #533
base: master
Are you sure you want to change the base?
Conversation
A common use case is where a user would like to construct a path and populate its corresponding `GoStruct` at the same time. This helper obviates the need to retrieve the struct using the `GetOrCreateXXX` functions on top of using the path API.
if err := goPathStructTemplate.Execute(&structBuf, structData); err != nil { | ||
return GoPathStructCodeSnippet{}, util.AppendErr(errs, err) | ||
} | ||
if err := goPathAndStructHelperTemplate.Execute(&structBuf, structData); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should generating the path and struct helper be optional? In general we've tried to have these other features that are new types of output flag-controlled in the library - even if we have it as on by default, it'd probably be nice to have something whereby a user can turn this off if they don't want it to save on generated code size.
@@ -2040,6 +2075,11 @@ type List struct { | |||
*ygot.NodePath | |||
} | |||
|
|||
// PathAndStruct returns the path struct and an empty oc.List for the path "/root-module/list-container/list". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting -- for YANG container
types it seems pretty clear to just return the struct, but in the case I call this with pathlib.ListContainer().ListAny().PathAndStruct()
should I get back oc.List{}
or map[...]*oc.List{}
?
One alternative is for Replace on an intermediate node to always construct a new value and accept a lambda that can modify it, e.g.
Not advocating for it, just thinking through the options. |
Wen, is this change defunct at this point -- or should we try and merge it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think maybe we should include this in the space of changes that will be considered for a merged/revised gNMI API in H1. At this point, this feels a little too much of a piecemeal contribution to me, and we might benefit from incorporating it into the more holistic look that is planned.
When generics is ready a better design for this might be just to provide the function func ygot.GoStruct[T GoStruct](PathStruct[T]) T {} |
A common use case is where a user would like to construct a path and populate its corresponding
GoStruct
at the same time. This helper obviates the need to retrieve the struct using theGetOrCreateXXX
functions on top of using the path API.This can be convenient for long paths such as the following example:
Before
After
This helper is not generated for leaves, since their types can be accessed using
ygot
helper functions such asygot.Int32()
.