-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
NUTCH-3032 Code for an ArbitraryIndexingFilter to index values resolved by user POJO code at index time #810
Conversation
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.
Hi @CatChullain nice patch!
Please see my minor comments. I have not tested this yet but will get around to that soon :)
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Show resolved
Hide resolved
src/plugin/index-arbitrary/src/test/org/apache/nutch/indexer/arbitrary/Echo.java
Show resolved
Hide resolved
src/plugin/index-arbitrary/src/test/org/apache/nutch/indexer/arbitrary/Multiplier.java
Show resolved
Hide resolved
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.
HGi @CatChullain please also address my feedback on logging statements. Thank you. This applies for the entire PR. Thanks
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
Thanks, Lewis! I got some of it done today. I'll consolidate the LOG statements a bit more tomorrow. |
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.
Hi @CatChullain please see some updated comments RE: logging.
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
...gin/index-arbitrary/src/java/org/apache/nutch/indexer/arbitrary/ArbitraryIndexingFilter.java
Outdated
Show resolved
Hide resolved
Thanks, Lewis! I moved all four to DEBUG, but I wonder where might be good spots for INFO level messages. I'm thinking of the operator or tech who doesn't dig into code and has an issue in the config. During dev & test myself, I sometimes forgot to increment the index.arbitrary.function.count and the plugin ignored the later fields. Just outputting that count value, and maybe something when overwrite is true, might be helpful for alerting someone that the config might not be what they'd believed. Do either of those (or something else) seem worthwhile, or does it make more sense to let people use it and see what issues they raise? |
@CatChullain thanks for your patience whilst we work this one 👍
The reason I suggested that the log level be revised from
|
Hi @CatChullain I associated this Jira ticket to the 1.20 release and made you assignee 👍 |
Thanks again, @lewismc. I did add those INFO messages, but I found an extra call to setIndexedConf from setConf that the filter() method handles more cleanly, so I removed that, too. |
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.
+1 thanks @CatChullain for considering all the feedback. Will merge once CI passes.
This is the initial code for an arbitrary indexing filter, NUTCH-3032.
It could be helpful to let end users manipulate information at indexing time with their own code without the need for writing their own indexing plugin. I mentioned this on the dev mailing list (https://www.mail-archive.com/dev@nutch.apache.org/msg31190.html) with some description of my work in progress.
One potential use is to address some of the same concerns that NUTCH-585 discusses regarding an alternative approach to picking and choosing which content to index, but this approach would allow making index time decisions, rather than setting the configuration for all content at the start of the indexing run.
Ideally a solution to NUTCH-585 would work at parse time, but this index time code still has potential uses for any kind of look-ups or calculations that depend on values in the document where the need to manipulate data exceeds what something like Jexl filter can do easily, or where outside data is worth incorporating into the document for use after indexing.
Thanks for your contribution to Apache Nutch! Your help is appreciated!
Before opening the pull request, please verify that
NUTCH-XXXX
)[NUTCH-XXXX] Issue or pull request title
)ant clean runtime test
LICENSE-binary
andNOTICE-binary
updated accordingly?We will be able to faster integrate your pull request if these conditions are met. If you have any questions how to fix your problem or about using Nutch in general, please sign up for the Nutch mailing list. Thanks!