-
Notifications
You must be signed in to change notification settings - Fork 2.9k
NIFI-5902 Added GeoEnrichIPRecord. #3231
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
Conversation
16ec7ff to
2f06cba
Compare
|
@markap14 @pvillard31 could one of your review? |
|
@pvillard31 @mattyb149 @zenfenan any chance of a review? |
|
@pvillard31 @bbende could one of you review? FWIW, we're actually using this processor at my client site to enrich very large CSV data sets. |
mattyb149
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some functionality provided by this that isn't available in IPLookupService? I thought you could use either the commercial or free Maxmind DB for either? If not, would it be better to augment the current one? If a new processor is necessary that's fine, just want to make sure we don't have too many equivalent ways of doing things.
nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/pom.xml
Outdated
Show resolved
Hide resolved
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Outdated
Show resolved
Hide resolved
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Show resolved
Hide resolved
|
@mattyb149 thanks, I'll try to get to these tonight! |
NIFI-5902 Updated GeoEnrichIPRecord with more error handling.
|
Done. |
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Show resolved
Hide resolved
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Outdated
Show resolved
Hide resolved
|
@mattyb149 I think we're ready to merge now. |
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Show resolved
Hide resolved
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Outdated
Show resolved
Hide resolved
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Show resolved
Hide resolved
|
Thanks @mattyb149 I'll try to get these done soon. |
|
Changes pushed @mattyb149 |
...undle/nifi-enrich-processors/src/main/java/org/apache/nifi/processors/GeoEnrichIPRecord.java
Outdated
Show resolved
Hide resolved
|
@mattyb149 made the change |
|
+1 LGTM, ran unit tests and tried various paths on a live NiFi instance, behavior is as expected and described. Thanks Mike for sticking with this! Merging to master |
|
Thanks! I'd like to do the ISP one as well, but I'm going to have to find someone willing to buy me the database ;) |
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:
For documentation related 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.