Skip to content
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

Add Script File feature #902

Merged
merged 5 commits into from
Aug 11, 2015
Merged

Add Script File feature #902

merged 5 commits into from
Aug 11, 2015

Conversation

nicolassing
Copy link
Contributor

Hello,

I've added scripting file-base feature in addition of inline scripting.

{
/**
* @param array|null $params
* @param null $id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@param string $id

@im-denisenko
Copy link
Contributor

@nicolassing Looks good, thanks.
Build failed, but this is not related to PR.
Could you please add a couple of functional tests and run cs fixer using make lint before merge?

@ruflin
Copy link
Owner

ruflin commented Aug 10, 2015

@nicolassing Any updates here? I plan to do a release today and would like to include this. Make sure merge master as quite a few changes went into master.

@nicolassing
Copy link
Contributor Author

@ruflin I've added a functional test. But this functional test will not work without the script file. I've updated elasticsearch configuration and created a script file (calculate-distance.groovy) in test/data/ but this script file must be in /etc/elasticsearch/scripts. I don't know how to do this with Docker and Vagrant :(

@ruflin
Copy link
Owner

ruflin commented Aug 11, 2015

Thx, I will have a look.

@ruflin ruflin mentioned this pull request Aug 11, 2015
@ruflin
Copy link
Owner

ruflin commented Aug 11, 2015

@nicolassing After fighting a little bit with ansible I got it running here: #914 I had to move the files from the data to the config/scripts directory as elasticsearch loads the files from there.

@ruflin ruflin merged commit 17383f9 into ruflin:master Aug 11, 2015
@ruflin
Copy link
Owner

ruflin commented Aug 11, 2015

I merged my pull request with the changes. Thanks for the contribution.

@nicolassing
Copy link
Contributor Author

👍 Someday i'd learn how to really use ansible, vagrant, docker, etc ... Someday ... 😄

@ruflin
Copy link
Owner

ruflin commented Aug 11, 2015

You can directly jump to docker ;-)

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