Add support for enums in templates

[geeknoid]

We want to increase usability by supporting enums as a first class thing in templates, allowing better YAML for the operator with better docs.

[guptasu]

So, @ozevren and I chatted about this a bit. Until we have support for first class enums in CEL where enums can be written as named things, we can ask the operator to write equivalent integer value for the enum values. In the generated code we will transform the integer value from the parsed expression into the enum value and pass it to the adapter. So, adapters will get nice enum value inside the Instance object but operators, for now, will write integers instead of enum's string name (when CEL will support enums to be written as strings, we can also let operators do the same thing).

@geeknoid WDYT ?

[geeknoid]

Hmmm. What we have now is that the operator specifies:

color: "blue"

I'd like it to be: in template: enum { RED, GREEN, BLUE} in config: color: BLUE

and you're suggesting that we do temporarily:

color: 2

Seems like a regression to me, I'd rather go with a mnemonic string than a naked int.

[guptasu]

Hmm... As long as template writers don't change the name of the Enum values we can even go with strings.

template

enum Color { RED, GREEN, BLUE}
message Template {
  Color color = 1
}

config:

color: (some.attribute == "xyz") ? "BLUE" : "RED" // just an exmpl. I know we don't have ternary opt.

and adapters get

type Color int

const (
        RED Color = iota
        GREEN
        BLUE
)
type Instance struct {
  color Color
}

@geeknoid @ozevren Thoughts now ?

[geeknoid]

So we get the right thing in the template and in the adapter, but the config format is a bit awkward. It sounds reasonable.

Do you think if/when we actually support enums, we could continue to support this proposed form in a compatible way? That is, can we roll out real enums without breaking these hacky enums?

On Wed, Jan 10, 2018 at 4:58 PM, Sunny Gupta [email protected] wrote:

Hmm... As long as template writers don't change the name of the Enum values we can even go with strings.

template

enum Color { RED, GREEN, BLUE} message Template { Color color = 1 }

config:

color: (some.attribute == "xyz") ? "BLUE" : "RED" // just an exmpl. I know we don't have ternary opt.

and adapters get

type Color int

const ( RED Color = iota GREEN BLUE ) Instance type { color Color }

@geeknoid https://github.com/geeknoid @ozevren https://github.com/ozevren Thoughts now ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/istio/old_mixer_repo/issues/1479#issuecomment-356789471, or mute the thread https://github.com/notifications/unsubscribe-auth/AVucHTmVrX3rZK2L3sGPMOjX8jaOEWD-ks5tJVyygaJpZM4QAtTV .

[guptasu]

I think so. We currently load every entry in config as string. Once we have real enums working in expression parser we can do something like this in the generated code..

if field.typeInfo == TypeInfo.Enum {
  inst.color, err := eval.EvalEnum(fieldExprStr, /*Might need enum descriptor or something here or during eval init time*/)
  if err != nil {
    colorStr, err := eval.EvalString(fieldExprStr)
    // transform colorStr to Enum properEnumColor
    ...
    ...

    inst.color = properEnumColor
  }
}

Thinking about this, even the hacky enum is not free and might take around ~2 weeks. So, depending on what @ozevren think proper enums can be supported in CEL, we can decide to go with the hack or not.