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-5922: Stateless NiFi, an alternative runtime for NiFi flows #3241

Closed
wants to merge 10 commits into from

Conversation

SamHjelmfelt
Copy link
Contributor

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.

@SamHjelmfelt
Copy link
Contributor Author

SamHjelmfelt commented Jan 3, 2019

A pre-built docker image can be found on docker hub with this tag: samhjelmfelt/nifi-fn:latest

@@ -0,0 +1 @@
target/
Copy link
Contributor

Choose a reason for hiding this comment

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

I think traditionally we just inherit the root .gitignore file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. removed.

nifi-fn/pom.xml Outdated

<groupId>org.apache.nifi.fn</groupId>
<artifactId>nifi-fn</artifactId>
<version>1.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you would want to change this to three-component versioning as we follow semantic versioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Updated.

@SamHjelmfelt
Copy link
Contributor Author

Here is a Apache OpenWhisk quick-start that comes preconfigured for NiFi-Fn. I will work to improve the documentation of this runtime option.
https://github.com/SamHjelmfelt/OpenWhisk-YarnDeployment

Note that this project also has support for OpenWhisk on YARN, making event-driven, ephemeral NiFi-Fn flows on Hadoop possible.

nifi-fn/pom.xml Outdated
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>org.apache.nifi.fn</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the groupId should just be org.apache.nifi unless you expect to have a number of artifacts here.

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. Fixed.

nifi-fn/pom.xml Outdated

<properties>
<java.version>1.8</java.version>
<nifi.version>1.9.0-SNAPSHOT</nifi.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

should just copy/paste 1.9.0-SNAPSHOT. this approach to property based version management fights with the maven release versions handling so it should be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense. fixed

@ottobackwards
Copy link
Contributor

Is there any design documentation on this? There is no javadoc


@Override
public Map<PropertyDescriptor, String> getProperties() {
return new HashMap<>(this.properties);
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that properties is injected in and never modified, we should probably just use this.properties = Collections.unmodifiableMap(properties); in the constructor and avoid copying the map here. As-is, this can result in quite a lot of garbage.

@SamHjelmfelt
Copy link
Contributor Author

Bringing in changes from @markap14. Still more to be done such as docker.

@ottobackwards
Copy link
Contributor

Still no design doc. The documentation in the readme or somewhere else should do more than describe the parameters of this module, they should document what it addresses and why you would use it.

"There are times when you want to do ${THING} and an environment like %{ENV}, but using NIFI ${PROBLEMS}. Nifi-FN addresses this by adding the ability to ${SIZZLE} and .........."

@SamHjelmfelt SamHjelmfelt changed the title NIFI-5922: NiFi-Fn, an alternative runtime for NiFi flows NIFI-5922: Stateless NiFi, an alternative runtime for NiFi flows Apr 8, 2019
@alopresto
Copy link
Contributor

Still no design doc. The documentation in the readme or somewhere else should do more than describe the parameters of this module, they should document what it addresses and why you would use it.

"There are times when you want to do ${THING} and an environment like %{ENV}, but using NIFI ${PROBLEMS}. Nifi-FN addresses this by adding the ability to ${SIZZLE} and .........."

Sam has shared this with a few of us offline and it's a very compelling story. Hopefully he can condense it into an appropriate form to share here.

@joewitt
Copy link
Contributor

joewitt commented Apr 8, 2019

how is this different than was in his proposal email thread?

we have that thread, the jira, a pr with lots of code and collab going...

@alopresto
Copy link
Contributor

Sorry, I was referring to the mailing list email. I think with the provided code, the email has more context and makes sense. I remember reading the email and not understanding the environment this would fit into.

@SamHjelmfelt
Copy link
Contributor Author

A copy of the latest docker image can be found here: samhjelmfelt/nifi-stateless:1.10.0-SNAPSHOT. It is 1.71GB, and the NARs are already un-packed. The API is the same as before and documented in the README file.

Not sure what is needed to push this into apache/nifi-stateless on docker hub.

@markap14
Copy link
Contributor

@SamHjelmfelt I know this has taken a while to get everything resolved, but it's a great contribution! Many thanks for sticking with it until we got it all sorted out. I've now merged to master! There's still plenty to do here, of course, but I've run several flows with it, and I think it's in good enough shape to go ahead and merge. Thanks again!

@SamHjelmfelt
Copy link
Contributor Author

Merged!

@SamHjelmfelt SamHjelmfelt deleted the nifi-fn branch May 24, 2019 00:09
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.

5 participants