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

bake: prototype for composable bake attributes #2758

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jsternberg
Copy link
Collaborator

This allows using either the csv syntax or a map to specify certain attributes.

@jsternberg
Copy link
Collaborator Author

Very incomplete first draft. This requires some changes to the gohcl package in the hcl package for it to work. The biggest thing is overriding the ImpliedType method and copying the full implementation from go-cty so that we can customize the behavior for certain types. In this case, I create a FromCtyValue interface and that works for our purposes.

The only changes to gohcl is creating a DecodeOptions which allows us to inject our own version of ImpliedType into the decoder. I also made it so we can change convert, but modifying the convert implementation was a lot more involved than modifying ImpliedType and using the existing capsule type.

bake/types.go Outdated
// ParseExports expects that format and it's easier to do that then reimplement it
// in multiple locations.
var sb strings.Builder
for name, val := range conv.AsValueMap() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't encode objects to csv string. That is backwards as the (typed) object is what we need.

If you want to reduce duplication then there can be generic csv -> map[k]v convert and then map to struct, although not sure if worth it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the prototype so the map[k]v version is the canonical form.

To reduce duplication, I've created a separate ExportEntry type with the same attributes. This type will implement encoding.TextUnmarshaler and json.Unmarshaler to control the marshaling and unmarshaling process in a consistent way. The type also has a ToPB() method to convert the type to the controller API version.

The csv parsing has been extracted into UnmarshalText and the original ParseExports method has been changed to be a loop that does this:

var out ExportEntry
if err := out.UnmarshalText([]byte(s)); err != nil {
    return nil, err
}
outs = append(outs, out.ToPB())

The a FromCtyValue method has been created so we can still detect it. But, we now only need one implementation that uses UnmarshalText and JSON unmarshaling and we should be able to reuse this implementation for each of these types.

This allows using either the csv syntax or a map to specify certain
attributes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants