feat: json dump funcs for cel #19
Conversation
yashmehrotra
force-pushed
the
cel-json-dump-funcs
branch
from
e7fa402
to
4f4febe
Compare
| if err != nil { | ||
| return types.String(err.Error()) | ||
| } |
There was a problem hiding this comment.
This is intentional, no other way to propagate the error to the users
celext/celfuncs.go
Outdated
| @@ -58,6 +66,46 @@ func k8sIsHealthy() cel.EnvOption { | |||
| ) | |||
| } | |||
|
|
|||
| func jsonArrayDump() cel.EnvOption { | |||
| return cel.Function("data.JSONArrayDump", | |||
There was a problem hiding this comment.
can we remove the prefix and use camelCase e.g. toJson ? with type overloading ?
Don't we already have this method from gomplate e.g. https://github.com/flanksource/gomplate/blob/main/funcs/data_gen.go#L29 ?
There was a problem hiding this comment.
Don't we already have this method from gomplate e.g. https://github.com/flanksource/gomplate/blob/main/funcs/data_gen.go#L29 ?
This does not work as we want it to, we want to return a string type and the function should accept a dyn type which then needs to be converted to map[string]any. Also, that function returns the wrong result. It converts the dyn type to string incorrectly
can we remove the prefix and use camelCase e.g. toJson ? with type overloading ?
cool, will try that
There was a problem hiding this comment.
Why do we need the dyn in the first place?
If those functions don't work, we should add tests for them and then exclude them so that we know its WIP
There was a problem hiding this comment.
Why do we need the dyn in the first place?
Its not a hard requirement. dyn supports functions like map, filter etc. so as a user it is easy to work with that. We can accept a any type as well here as well, it wont break anything
If those functions don't work, we should add tests for them and then exclude them so that we know its WIP
Have created an issue to track that: #20
No description provided.