Skip to content

NIFI-5895 - Add a processor to start an Oozie workflow#3218

Closed
pvillard31 wants to merge 4 commits intoapache:masterfrom
pvillard31:oozie
Closed

NIFI-5895 - Add a processor to start an Oozie workflow#3218
pvillard31 wants to merge 4 commits intoapache:masterfrom
pvillard31:oozie

Conversation

@pvillard31
Copy link
Contributor

@pvillard31 pvillard31 commented Dec 14, 2018

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:

  • Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
  • Have you written or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
  • If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
  • If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

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.

@pvillard31
Copy link
Contributor Author

pvillard31 commented Dec 14, 2018

This PR has been tested against an Oozie 4.2.0 server instance in the following conditions:

  • Plaintext
  • Plaintext + SSL
  • Kerberos
  • Kerberos + SSL

A very basic example has been used:
https://github.com/apache/oozie/tree/branch-4.3/examples/src/main/apps/shell

NiFi workflow:
https://gist.github.com/pvillard31/4a0a7b33dc27c11538cec0b5e80c0324

The workflow.xml from the Oozie repo must be pushed in HDFS in a directory and that directory is used in the configuration of the processor. All the properties given in the job.properties file from the Oozie repo must be passed through the configuration of the processor or the attributes of the flow files.


public static final PropertyDescriptor APP_PATH = new PropertyDescriptor.Builder()
.name("oozie-app-path")
.displayName("HDFS Application Path")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend it to be changed to something like Workflow Path.

<packaging>jar</packaging>

<dependencies>
<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Pure nitpick: Would like to see proper indentation. :)

@pvillard31
Copy link
Contributor Author

Thanks for the review @zenfenan - just pushed a commit to address your comments.

String principal = credentialsService.getPrincipal();
String keyTab = credentialsService.getKeytab();
Configuration conf = new Configuration();
conf.set("hadoop.security.authentication", "kerberos");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any other Oozie-or HDFS-related properties that should come from an oozie-site.xml or other site config file? Just wondering if there should be a property like the other Hadoop processors to specify configuration file(s).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not to my knowledge - IIRC this Kerberos property is to enable SPNEGO authN against the Oozie server REST API but there should not be any other requirement. The files hdfs-site, core-site, etc, are loaded by the Oozie server anyway.


<properties>
<oozie.version>4.3.1</oozie.version>
<hadoop.version>3.0.0</hadoop.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, we should probably document that hadoop.version should match the one in nifi-hadoop-libraries, unless there's some way to inherit it (see my comment in the NAR POM)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep true. Didn't look too long (switching planes in airport) but didn't find a way to inherit the version from the nifi-hadoop-libraries so I just added a comment about the version.

<dependencies>
<dependency>
<groupId>org.apache.nifi</groupId>
<artifactId>nifi-standard-services-api-nar</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, shouldn't we have nifi-hadoop-libraries-nar as a parent here? Then the hadoop-common and other dependencies in the processors POM could be marked as provided and we'd be sure to be using the same versions across the Hadoop-related processors.

@@ -0,0 +1,136 @@
nifi-kafka-nar
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy/pasta typo here, should be nifi-oozie-nar. Did you check the L&N to make sure it's correct for Oozie (vs wherever it came from)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch - I tried to be exhaustive on the Oozie deps, hopefully didn't miss one.

@joewitt
Copy link
Contributor

joewitt commented Mar 25, 2021

closing due to inactivity

@joewitt joewitt closed this Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants