Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIFI-2020 - Enhance JoltTransformJSON processor to support custom transforms #564

Closed
wants to merge 1 commit into from

Conversation

YolandaMDavis
Copy link
Contributor

@YolandaMDavis YolandaMDavis commented Jun 23, 2016

This is the initial commit to enhance the JoltTransformJSON processor by providing the ability to support custom transformations (that implement Jolt's Transform Interface) via drop in jars/libraries.

The below describes the rules behind this enhancement:

Case #1 – Custom Transform Selected

In this case if the Custom option of Transform DSL is selected then 1) a Custom Transform Class Name should be entered and 2) one or more module paths should be provided. The Custom Transform Class Name should be a fully qualified classname (e.g. eu.zacheusz.jolt.date.Dater). The Module Path can take a comma delimited list of directory locations or one or more jar files. Once these fields are populated the Advanced view will support validation and saving of the specification. A user can switch between transformation types in the UI but not custom class names & module paths.

Case #2 – Chain Transformation Selected with Custom Transformations embedded

In this case if a user wants to use one or more transforms (that include custom transformations) in their specification then the Chainr spec option can be used with one or more module path provides. In this case the Custom Transform Class Name property is not required and would be ignored (since one or more custom transformations could be invoked in the spec). As in Case #1 the Advanced view will support validation and saving of specification if the required fields are populated for this case.

To aid in testing, the Jolt Date custom transformation project can be used:

A jar file can be built from this project and used for testing

Also a readme file with example specification for jolt-date is attached along with snapshots of configuration setup and execution.
README.txt
nifi-2020_custom_configuration
nifi-2020_advancedui_custom

@YolandaMDavis
Copy link
Contributor Author

Just to add I'm thinking of incorporating ContextualTransform support once Variable Registry is incorporated to provide the transformation access to system/env properties, flow file attributes, and custom properties. Variable Registry is currently implemented in 0.7

@YolandaMDavis
Copy link
Contributor Author

@davetorok here is the first phase of custom transform support (supporting the transform interface)

}

if(transformName.equals("jolt-transform-custom")) {
transform = TransformFactory.getCustomTransform(customClassLoader,specificationDTO.getCustomClass(), specJson);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that customClassLoader would be null here? I see you check for that in the "else" clause but not the "if" clause. Even if the UI / customValidators prevent this kind of thing, it might still be a good thing to add. If there is a way to directly make the API call without a value for "modules", then this could cause an NPE and a unit test could show that the added null check works :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed this code should be improved in general, which I'll take care of. The case I should be covering here is the fact that customClassLoader could be used with a custom transform or other transforms (such as a chain which can contain custom transformations). The code assumes that if you selected custom you should have a module set which isn't good to do. I'll update and get it out here.

@mattyb149
Copy link
Contributor

Since the test JARs are built from source code (in test/java), we're probably fine to just include them, but I wasn't sure if there's any paperwork needed. @joewitt Think these are fine as-is? I believe so but wanted a second opinion. An alternative could be to set the Module Directory to point at the target/classes directory, that should allow them to be loaded but not have to be in a JAR.

public static final PropertyDescriptor CUSTOM_CLASS = new PropertyDescriptor.Builder()
.name("jolt-custom-class")
.displayName("Custom Transformation Class Name")
.description("Fully Qualified Class Name for Custom Transformation Module Directory should be specified")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's two sentences so should have a period in between. Also the second part could be elaborated upon, since the Module Directory shouldn't need to be specified (if their JAR is somehow already in the NiFi classpath), rather the class just needs to be able to be loaded by the processor by any means. Not sure how to word that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just took that second part out and will elaborate in the additionalDetails page and a bit on the custom module directory description.

@YolandaMDavis
Copy link
Contributor Author

@mattyb149 just pushed commits (no rebase so that you can easily see changes). I wanted to get your input on the stream/lambdas implementation I have in ClassLoaderUtils...not too happy with it :(. Unfortunately lambda's don't handle checked exceptions too well, so I'm forced right now to wrap in a RuntimeException when attempting to extract the URL from a URI (it throws MalformedURLException potentially) which is no bueno imo. Any thoughts on that or alternatives?

@mattyb149
Copy link
Contributor

I had forgotten about the checked exception thing. If the lamdbas/streams are just as awkward as Java 7 code (due to the exceptions, etc.) then there's probably no need to force it. Everything else looks good, I still need to build and run it, but if you anticipate another change I can wait for that (might as well rebase then too)

@YolandaMDavis
Copy link
Contributor Author

@mattyb149 rebased and rolled back the lambda on the ClassLoaderUtil. Should be good for your tests when you're ready.

NIFI-2020 - updates to use lambdas/stream wherever possible and fix potential nullpointer issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants