NIFI-4130 Add lookup controller service in TransformXML to define XSL… #1953
Conversation
pvillard31
force-pushed
the
xslt-text-property
branch
from
dccf155
to
6f12f4c
Compare
|
@pvillard31 It seems like we should just use LookupAttribute for this use case. Thoughts? |
|
Yes @jfrazee, that could be an option. However, do you think that's a good idea to have the XSLT as an attribute of the flow files? The XSLT could be really big and I'm not sure users will have the reflex to use an UpdateAttribute to remove it after the TransformXml. |
|
@pvillard31 Right, wouldn't want to do that. I misunderstood what you were doing. This makes sense. |
|
Reviewing... |
pvillard31
force-pushed
the
xslt-text-property
branch
from
6f12f4c
to
1a8035c
Compare
|
Hey @bdesert - I just rebased the PR. I'll probably add a warning about adding large XSLTs in a StringLookupService as it'll be in memory. |
There was a problem hiding this comment.
Although JUnits aren't failing and look good (covering all the use cases), actual implementation is failing, when cache is in use. It works fine when cache is 0.
Exception I'm getting:
Caused by: java.lang.NullPointerException: null
at org.apache.nifi.processor.StandardProcessContext.getControllerService(StandardProcessContext.java:137)
at org.apache.nifi.attribute.expression.language.StandardPropertyValue.asControllerService(StandardPropertyValue.java:187)
at org.apache.nifi.processors.standard.TransformXml.newTemplates(TransformXml.java:270)
at org.apache.nifi.processors.standard.TransformXml.access$000(TransformXml.java:93)
I think it is related to cache creation in onSchedule and context.
Looks like in Junits and in actual running app, this is different.
| .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) | ||
| .addValidator(StandardValidators.FILE_EXISTS_VALIDATOR) | ||
| .build(); | ||
|
|
||
| public static final PropertyDescriptor XSLT_CONTROLLER = new PropertyDescriptor.Builder() | ||
| .name("xslt-controller") | ||
| .displayName("XSLT controller") |
There was a problem hiding this comment.
"XSLT Lookup" Would be more readable.
Description: "Lookup controller used to store..."
And: XSLT_CONTROLLER_KEY: "XSLT Lookup Key" (description looks fine)
| + "the XSLT controller property.") | ||
| .required(false) | ||
| .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES) | ||
| .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) |
There was a problem hiding this comment.
since this property supports EL, shouldn't it be NON_EMPTY_EL_VALIDATOR?
|
Hey @bdesert - thanks a lot for the review, I just pushed a commit that should address your comments. |
pvillard31
force-pushed
the
xslt-text-property
branch
from
09cde47
to
435a73d
Compare
|
+1 LGTM. Tested on local env, with XSLT as a file (regression), with lookup service, cache size 0 and >0, all works as expected. Ready for merge. @mattyb149 , please could you please give a final look? |
|
+1 LGTM, thanks for the review @bdesert and the improvement @pvillard31 ! Merging to master |
…T from the UI addressed review comments Signed-off-by: Matthew Burgess <mattyb149@apache.org> This closes apache#1953
…T from the UI addressed review comments Signed-off-by: Matthew Burgess <mattyb149@apache.org> This closes apache#1953
…T from the UI
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:
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.