Skip to content

NIFI-2615 Adding support for GetTCP processor#1433

Closed
apsaltis wants to merge 1 commit intoapache:masterfrom
apsaltis:NIFI-2615
Closed

NIFI-2615 Adding support for GetTCP processor#1433
apsaltis wants to merge 1 commit intoapache:masterfrom
apsaltis:NIFI-2615

Conversation

@apsaltis
Copy link
Contributor

This is a major refactoring of a previous version of this processor that was never merged due to poor design. Special thanks to @olegz for all of his help in refactoring this processor to bring it inline with NiFi best practices.

This is a major refactoring of the previous version of this processor.
@mosermw
Copy link
Member

mosermw commented Jan 19, 2017

Nice contribution, @apsaltis. Does this need to be in its own nifi-gettcp-bundle? PutTCP is in the nifi-standard-bundle and I would expect GetTCP and PutTCP to be located in the same NAR.

@apsaltis
Copy link
Contributor Author

@mosermw -- I can totally see how this processor would be at home in the same NAR, the reasoning for keeping it separate was to have one less processor to modify and hopefully ease the effort to integrate it into the extension registry [1] when that is ready.

[1] https://cwiki.apache.org/confluence/display/NIFI/Extension+Repositories+%28aka+Extension+Registry%29+for+Dynamically-loaded+Extensions

@mosermw
Copy link
Member

mosermw commented Jan 19, 2017

That's definitely a fair reason. I believe this would be the first NAR whose name contains the name of a processor. Would you consider changing the name to nifi-tcp-bundle and nifi-tcp-nar? When the extension registry is ready, then we could move PutTCP into this nar?

@olegz
Copy link
Contributor

olegz commented Jan 19, 2017

@mosermw +1 to the name change!

@apsaltis
Copy link
Contributor Author

apsaltis commented Jan 19, 2017

@mosermw Changing the name certainly makes sense to me. Honestly had not dawned on my that it was the first processor whose name matches the nar.

I will close this PR and then re-open one with the new name.

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.

3 participants