Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Bulk scripting support to RabbitMQ river #18

Closed
wants to merge 8 commits into from

Conversation

sebaes
Copy link

@sebaes sebaes commented Mar 26, 2013

Added bulk scripting support to RabbitMQ river:

curl -XPUT 'localhost:9200/_river/my_river/_meta' -d '{
    "type" : "rabbitmq",
    "rabbitmq" : { },
    "bulk_script_filter" : {
        "script" : "mock_script",
        "script_lang" : "native"
    }
}'

Have a look at Native (Java) Scripts documentation for how to declare a script.

Relative to #17.

@ghost ghost assigned dadoonet May 2, 2013
@otisg
Copy link

otisg commented May 2, 2013

@dadoonet any chance you can merge this so we can point our dependency to the real RabbitMQ River repo instead of our clone?

@dadoonet
Copy link
Member

dadoonet commented May 2, 2013

I'm on it. I just want to add a testcase and will push it.

@dadoonet
Copy link
Member

dadoonet commented May 3, 2013

Hey @otisg @sebaes

How do you use scripts with a message body as:

{ "index" : { "_index" : "twitter", "_type" : "tweet", "_id" : "1" } }
{ "tweet" : { "text" : "this is a tweet" } }
{ "delete" : { "_index" : "twitter", "_type" : "tweet", "_id" : "2" } }
{ "create" : { "_index" : "twitter", "_type" : "tweet", "_id" : "1" } }
{ "tweet" : { "text" : "another tweet" } } 

Can you give an example of how you will build it:

curl -XPUT 'localhost:9200/_river/my_river/_meta' -d '{
    "type" : "rabbitmq",
    "rabbitmq" : { },
    "script_filter" : {
        "script" : "body. ....",
        "script_lang" : "mvel"
    }
}'

Thanks

@sebaes
Copy link
Author

sebaes commented May 3, 2013

Hi David,

I have plagiarized your RabbitMQRiverTest, which by the way has a missing closing bracket in line 52, and created a test for the scripting functionality.

The format received is not nice, due to bulk header + optional body, but it's possible to parse it, do some stuff, and flatten it again. I have gisted an example (and committed it too) that filters out "create" operations.
https://gist.github.com/sebaes/5513981

Let me know if you have any questions or concerns.

Thanks,
Sebastian.

@otisg
Copy link

otisg commented May 10, 2013

@dadoonet does what @sebaes work for you or is anything else needed?

@dadoonet
Copy link
Member

@otisg @sebaes Sorry. I was on holidays. :-D
Yes it answers on the native part. I was wondering if we can deal with bulk structure using mvel scripts as well? I can't see a way of doing it.

If it's not doable, I will add it to the README.

Thoughts?

dadoonet pushed a commit to dadoonet/elasticsearch-river-rabbitmq that referenced this pull request May 13, 2013
Closes #17.
Closes elastic#18.
dadoonet pushed a commit to dadoonet/elasticsearch-river-rabbitmq that referenced this pull request May 13, 2013
Closes #17.
Closes elastic#18.
@sebaes
Copy link
Author

sebaes commented May 13, 2013

Well, I am not much of a mvel expert myself, but for this use case it seems too much for a quick one-line script. Maybe it's doable with a lot of work, but probably even then not worth it.

Thanks for the integration!.

@dadoonet
Copy link
Member

dadoonet commented Jun 4, 2013

Regarding to this PR, I feel that we should do something different here than in other rivers.
We can probably support scripting doc by doc (when indexing) with the script_filter option as it works in any other river.

So, we probably need to put your PR under a new bulk_script_filter option. The script will hold all the bulk content as your example shown it and won't process document per document.

Make sense?
What do you think?

@sebaes
Copy link
Author

sebaes commented Jun 4, 2013

The new name bulk_script_filter makes sense to me.

So you plan to add a general option to use scripts with script_filter? Maybe the name is a little confusing with: http://www.elasticsearch.org/guide/reference/query-dsl/script-filter/
But it's the name CouchDB river uses now. Alternatives could be script_preprocess, indexing_script, ...?

@dadoonet dadoonet closed this in 3ea9f9f Jun 5, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants