Skip to content
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

RFC: Add Formatters #29

Open
amorenoz opened this issue Jan 27, 2021 · 3 comments
Open

RFC: Add Formatters #29

amorenoz opened this issue Jan 27, 2021 · 3 comments

Comments

@amorenoz
Copy link

Currently, the library does minimum data post-processing. This is a good approach, specially performance wise. However, it has the disadvantage of exposing ovsdb-specific formats (such as OvsSet and OvsMap) to the API clients.

I think it would be beneficial to be able to expose the data in more flexible ways. In order to keep backwards compatibility and access to the 'raw' data for clients that require it, I would like to propose adding another 'layer' of structs that can massage the output of OvsdbClient.
All Formatters should be able to ingest data coming from OvsdbClient such as Row and TransactResponse and return an arbitrary object with the post-processed data. Similarly, they should be able to consume data in arbitrary format and return structs consumed by OvsdbClient such as Mutation, Condition, or more generically, Operation. Unfortunately, since the returned types are different, it might not be possible to put all Formatters under an interface.

For instance, I can currently think of two possible Formatters:

NativeFormatter

This formatter could simply hide the ovs-specific fields such as OvsSet and OvsMap (not sure if it would be able to remove UUID since it would be difficult to distinguish from a normal string). For example:

struct NativeFormatter {}

// FormatRow simply removes OvsSet and OvsMap from the fields so they can safely be casted to native
// fields such as slices and maps.
func (f NativeFormatter) FormatRow(*libovsdb.Row) map[string] interface{}

// NewMutation creates a mutation from native types so you can create.
// example use:
// nf := NativeFormatter{}
// op := libovsdb.Operation {
// ...
// Mutations : nf.NewMutation("bridges", "insert", []string{"br0", "br1"})
// }
func (f NativeFormatter) NewMutation(column string, mutator string, value interface{}) []interface{}

DynamicFormatter

This Formatter could accept an additional pointer to a struct that would be populated based on dynamic type inference.
For example, we could define a ovs field tag that could be used by the formatter to determine how to populate the fields. For example:

type MyBridge struct {
    UUID            string      `ovs:"_uuid"`
    Name            string      `ovs:"name"`
    Controllers     [] string   `ovs:"controllers"`
}
struct DynamicFormatter {}

// FormatRow populates the object pointed by objPtr based on its 'ovn' tags
func (f NativeFormatter) FormatRow(row *libovsdb.Row, objPtr interface{}) error

I think having this functionality here in libovsdb could make it easier for clients to use it.
What do you think?

@amorenoz
Copy link
Author

@hzhou8 @vtolstov This could serve as base for the automatically generated APIs that we're discussing in eBay/go-ovn#29

@vtolstov
Copy link

vtolstov commented Feb 3, 2021

@hzhou8 i like approach with dynamic formatter, mostly naming are bad =)) i'm comment pr, thanks

@amorenoz
Copy link
Author

amorenoz commented Feb 3, 2021

@vtolstov I agree agree about the bad naming :)
Any suggestion?
s/Formatter/API/?
s/Dynamic/Typed/?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants