NEOS-570: custom code transformer - updated protos #988
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #988 +/- ##
==========================================
+ Coverage 58.52% 58.85% +0.33%
==========================================
Files 84 84
Lines 8581 8672 +91
==========================================
+ Hits 5022 5104 +82
- Misses 2939 2946 +7
- Partials 620 622 +2 ☔ View full report in Codecov by Sentry. |
backend/services/mgmt/v1alpha1/transformers-service/transformers.go
Outdated
Show resolved
Hide resolved
backend/services/mgmt/v1alpha1/transformers-service/transformers_test.go
Outdated
Show resolved
Hide resolved
| @@ -54,7 +56,7 @@ const ( | |||
| Passthrough Transformation = "passthrough" | |||
| Null Transformation = "null" | |||
| Invalid Transformation = "invalid" | |||
| Javascript Transformation = "javascript" | |||
| Javascript Transformation = "transform_javascript" | |||
There was a problem hiding this comment.
Probably want to change this property to TransformJavascript as well as I think we will also want a javascript generator.
There was a problem hiding this comment.
This doesnt look like it was resolved.
There was a problem hiding this comment.
I was referring to the property name itself.
| Javascript Transformation = "transform_javascript" | |
| TransformJavascript Transformation = "transform_javascript" |
backend/services/mgmt/v1alpha1/transformers-service/transformers.go
Outdated
Show resolved
Hide resolved
backend/services/mgmt/v1alpha1/transformers-service/transformers_test.go
Outdated
Show resolved
Hide resolved
| @@ -620,6 +640,14 @@ func parseJavascriptForColumnName(jsCode, targetWord, replacementWord string) st | |||
| return strings.ReplaceAll(jsCode, targetWord, replacementWord) | |||
| } | |||
|
|
|||
| func constructJavascriptCode(jsCode, col string) string { | |||
| if jsCode != "" { | |||
| return fmt.Sprintf(`function fn1(value){%s};const input = benthos.v0_msg_as_structured();const output = { ...input };output["%[2]s"] = fn1(input["%[2]s"]);benthos.v0_msg_set_structured(output)`, jsCode, col) | |||
There was a problem hiding this comment.
Please use newlines here. this is not very readable.
There was a problem hiding this comment.
Also this still isn't quite right. You need to take all of the columns that are using a js processor and build a single script that processes all of them at once. We dont want to split that across N processors. Minimize benthos overhead by having a single processor for handling javascript.
There was a problem hiding this comment.
it's producing this as an output:
processors:
- javascript:
code: |-
(() => {
function fnname(value) {
var payload = value+=" hello";return payload;
};
const input = benthos.v0_msg_as_structured();
const output = { ...input };
output["name"] = fnname(input["name"]);
benthos.v0_msg_set_structured(output);
function fnfirst_name(value) {
var payload = value+=" firstname";return payload;
};
const input = benthos.v0_msg_as_structured();
const output = { ...input };
output["first_name"] = fnfirst_name(input["first_name"]);
benthos.v0_msg_set_structured(output);
})();
where both transformers for different columns (name & first_name) are in the same javascript processor.
| (() => { | ||
| function fn1(value){ | ||
| var a = value + "test"; | ||
| return a }; | ||
| const input = benthos.v0_msg_as_structured(); | ||
| const output = { ...input }; | ||
| output["name"] = fn1(input["name"]); | ||
| benthos.v0_msg_set_structured(output); | ||
| })(); |
There was a problem hiding this comment.
We need a test here that shows what this looks like with multiple columns being transformed.
Also it would be great if this were formatted better.
There was a problem hiding this comment.
added a test to the benthos_builder_test file called Test_ProcessorConfigMultiJavascript
| r, w, _ := os.Pipe() | ||
| os.Stdout = w |
There was a problem hiding this comment.
idk about this. Looks like you are using the global Stdout to be this os.Pipe for a single test.
How does this work for the rest of the tests?
There was a problem hiding this comment.
i was trying to come up with a way to actually test the transformed value output and since benthos writes to stdout for codec:lines i'm piping that and then getting the resulting value. I wasn't able to find anywhere else where we were actually processing the benthos config and checking the output vs. just the formatting and structure of the benthos config. Open to other ideas.
There was a problem hiding this comment.
yea you're right we don't have any tests today that test the benthos output at an integration level.
A better approach here might be to have benthos output to a file in the temp directory that you can then read from afterwards to verify the output. That way there is no need to hijack the stdout.
evisdrenova
force-pushed
the
custom-code-transformer
branch
from
f5a1029
to
e821e43
Compare
No description provided.