NIFI-3536 Adding GetSOAP processor which supports generically invoking SOAP endpoints#1824
NIFI-3536 Adding GetSOAP processor which supports generically invoking SOAP endpoints#1824apsaltis wants to merge 1 commit intoapache:masterfrom
Conversation
pvillard31
left a comment
pvillard31
left a comment
There was a problem hiding this comment.
Hey @apsaltis, thanks for your contribution. I left some comments and it'll need to be rebased against master.
| .displayName("Endpoint URL") | ||
| .description("The endpoint url that hosts the web service(s) that should be called.") | ||
| .required(true) | ||
| .expressionLanguageSupported(false) |
There was a problem hiding this comment.
I think expression language should be supported on almost all the properties: with the variable registry, it can be used to externalize values that would be different between environments, also, since this processor can accept incoming flow files, this could be read from the attributes of the flow files.
There was a problem hiding this comment.
Makes total sense.
| @WritesAttribute(attribute = "getsoap.request.wsdl_url", description = "The request WSDL URL"), | ||
| @WritesAttribute(attribute = "getsoap.request.method", description = "The request method"), | ||
| @WritesAttribute(attribute = "mime.type", description = "Sets mime type to text/xml")}) | ||
| @DynamicProperty(name = "The name of a input parameter the needs to be passed to the SOAP method being invoked.", |
There was a problem hiding this comment.
Am I correct in stating you are referring to the bad grammar used in the value of the name parameter of the DynamicProperty on line 75?
There was a problem hiding this comment.
Great, will get it addressed.
| @WritesAttribute(attribute = "mime.type", description = "Sets mime type to text/xml")}) | ||
| @DynamicProperty(name = "The name of a input parameter the needs to be passed to the SOAP method being invoked.", | ||
| value = "The value for this parameter '=' and ',' are not considered valid values and must be escaped . " + | ||
| "Note, if the value of parameter needs to be an array the format should be key1=value1,key2=value2. ", |
| .dynamic(false) | ||
| .build(); | ||
|
|
||
| public static final Relationship REL_SUCCESS = new Relationship.Builder() |
There was a problem hiding this comment.
Since the processor accepts incoming flow files, wouldn't that make sense to also have a failure relationship? (in particular if some properties are evaluated against attributes of the incoming flow files)
There was a problem hiding this comment.
Good point. I agree.
| if (authHeader != null) { | ||
| serviceClient.addHeader(authHeader); | ||
| } | ||
| getLogger().info("Sending authHeader: " + authHeader); |
There was a problem hiding this comment.
What do you think about logging this at debug level? Otherwise it could be quite verbose when using the processor at a high rate.
There was a problem hiding this comment.
Makes total sense.
|
|
||
| @BeforeClass | ||
| public static void setup() { | ||
| mockServer = startClientAndServer(1080); |
There was a problem hiding this comment.
I think we should dynamically choose a port as we are doing in some other places instead of selecting 1080. What do you think?
There was a problem hiding this comment.
I think that makes total sense.
|
Closing due to inactivity. |
3 participants
This is a follow-on to P/R #1741 that I closed to both rebase off the 1.3.0-SNAPSHOT source and also update the NOTICE in nifi-assembly with the missing artifacts
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.