-
Notifications
You must be signed in to change notification settings - Fork 256
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create method to add SearchHit info to metadata #1034
Conversation
Signed-off-by: Mathieu Bret <mikwiss00@gmail.com>
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.
I am not against the idea but the implementation is not quite right, see comments
Thanks @Mikwiss
@@ -164,13 +164,19 @@ public void open( | |||
protected final boolean addHitToBuffer(SearchHit hit) { | |||
Map<String, Object> keyValues = hit.getSourceAsMap(); | |||
String url = (String) keyValues.get("url"); | |||
|
|||
Map<String, List<String>> metadatas = (Map<String, List<String>>) keyValues.get("metadata"); |
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.
what happens to the metadatas object afterwards? it would feel more logical if it did something to the actual metadata object which is being returned.
|
||
Map<String, List<String>> metadatas = (Map<String, List<String>>) keyValues.get("metadata"); | ||
addHitInfoToMetadata(metadatas, hit); | ||
|
||
// is already being processed - skip it! | ||
if (beingProcessed.containsKey(url)) { |
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.
Move that right after the line 166
String url = (String) keyValues.get("url");
|
||
Map<String, List<String>> metadatas = (Map<String, List<String>>) keyValues.get("metadata"); | ||
addHitInfoToMetadata(metadatas, hit); | ||
|
||
// is already being processed - skip it! | ||
if (beingProcessed.containsKey(url)) { | ||
return false; | ||
} | ||
return buffer.add(url, fromKeyValues(keyValues)); |
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.
I would store the value returned by fromKeyValues(keyValues) in a variable
and have addHitInfoToMetadata() operate on it and become
addHitInfoToMetadata(Metadata metadata, SearchHit hit)
Signed-off-by: Mathieu Bret <mikwiss00@gmail.com>
Thanks @jnioche ! You're right, it was a bit childish 馃憥 |
...ch/src/main/java/com/digitalpebble/stormcrawler/elasticsearch/persistence/AbstractSpout.java
Show resolved
Hide resolved
Thanks @Mikwiss |
Hi all !
We use ElasticSearch/AbstractSpout and we want to add some SearchHit info to metadata (es document id in our case).
Actually, we already do it with a
ForkCollapsingSpout
, but we rather not to fork a class only for that.We can also remove
final
toaddHitToBuffer(SearchHit hit)
instead of this new method, it's up to you ! 馃憤Thanks for all reviewers !
Signed-off-by: Mathieu Bret mikwiss00@gmail.com
Thanks for contributing to StormCrawler, your efforts are appreciated!
Developer Certificate of Origin
By contributing to StormCrawler, you accept and agree to the following terms and conditions (the Developer Certificate of Origin) for your present and future contributions submitted to StormCrawler.
Please refer to the Developer Certificate of Origin section in
CONTRIBUTING.md
for details.Before opening a PR, please check that:
Thanks!