feat: FEEL function for invoking outbound connectors #2744
Conversation
chillleader
requested review from
sbuettner,
jonathanlukas and
johnBgood
and removed request for
jonathanlukas
| String variablesJson; | ||
| try { | ||
| variablesJson = objectMapper.writeValueAsString(variables); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("Error while serializing variables", e); | ||
| } |
There was a problem hiding this comment.
If we transform the variables to json wont we have an issue with passing data around like a byte[] for example?
There was a problem hiding this comment.
Do you have a specific scenario in mind? As I'm thinking about it, it seems that we will be fine if we:
- only pass binary data as part of the
Documentclass, and - have a proper serialization/deserialization mechanism in place for
Documentthat converts it to base64.
I might be missing something, and it is possible that we can do secret replacement more efficiently 🤔
There was a problem hiding this comment.
@chillleader Sure, lets assume we want to execute an S3 connector in the resultExpression of the REST Connector to store the binary body. Passing a byte[] document wouldnt really be efficiently possible if we serialize it to json first and read it back again for larger files. Here it would be better to be able to directly invoke the connector passing the objects/references in memory to be able to use streams for example.
Yes, secret replacing would be an issue but this could theoretically be done by only replacing it when an object in the variables is of type string before passing it to the Connector.
There was a problem hiding this comment.
Thanks for clarifying, makes sense. This requires refactoring how we replace secrets in all outbound connectors to avoid having two secret replacement methods (we are still getting a json string from the zeebe job). I'm not opposed to that and can also attempt this as part of this PR.
There was a problem hiding this comment.
Great, you can also add secret support later on to figure out whether our approach here is reasonable 😄
...src/main/java/io/camunda/connector/runtime/feel/ConnectorInvocationFeelFunctionProvider.java
Outdated
Show resolved
Hide resolved
| Object jsonVariables) { | ||
| super(secretProvider, validationProvider, objectMapper, jsonVariables); |
There was a problem hiding this comment.
Can we rename jsonVariables to variables?
...c/main/java/io/camunda/connector/runtime/core/outbound/AbstractOutboundConnectorContext.java
Outdated
Show resolved
Hide resolved
johnBgood
force-pushed
the
epic-1748-doc-ref-handling
branch
from
0c3a8bb to
381da62
Compare
|
Closing this as per our decision to move along with a different approach. |
Description
This PR implements:
ExternalConnectorExecutorthat allows invoking outbound connectors outside of the job worker contextConnectorInvocationFeelFunctionProviderthat can be dynamically added to theFeelEngineWrapper(e.g. as a Spring bean)TransientDataStoreto store the binary data references away from serialization/FEEL engineRelated issues
closes https://github.com/camunda/team-connectors/issues/785