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

Should we remove file-based scripts and templates? #21798

Closed
clintongormley opened this issue Nov 25, 2016 · 8 comments · Fixed by #24627
Closed

Should we remove file-based scripts and templates? #21798

clintongormley opened this issue Nov 25, 2016 · 8 comments · Fixed by #24627
Labels
>breaking :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache feedback_needed help wanted adoptme

Comments

@clintongormley
Copy link
Contributor

We have largely removed support for file-based configuration (eg mappings, settings, index templates) in favour of storing this info in the cluster.

For scripts and mustache templates, we allow them to be passed inline or to be stored in the cluster state. We used to be concerned about locking down scripts in Groovy etc, but with Painless we have safe scripting. Are file based scripts now redundant?

The one upside of files is that scripts can be written with new lines and formatting, while with inline/stored scripts they have to be passed in in a single line of JSON with newlines escaped. However this could be solved by improving Console to do the multi->single line conversion automatically.

Any other reasons for keeping file scripts?

@rjernst
Copy link
Member

rjernst commented Nov 25, 2016

See #10959

@dadoonet
Copy link
Member

Totally agreed. People should use API to distribute their scripts in the cluster.
I also agree that improving Console to generated a single line version of a script is important to have before we definitely remove that feature.
It can be deprecated in 5.x IMO.

@clintongormley
Copy link
Contributor Author

I'm leaving this open for feedback from users - are there situations solved by having file based scripts which are not solved by stored scripts?

@tlrx
Copy link
Member

tlrx commented Nov 28, 2016

It know it is often used to create scripts with automated deployment tools like ansible or chef. These tools could use the REST API too but it is usually easier to copy files rather than waiting for the cluster to be green/put the script/handle retries and authentication.

We could maybe use a "drop in" folder where the user can copy the scripts and they will be loaded and added as stored script by the node itself? This would still allow to deploy scripts as simple files, and plugins during their installation could also add files in this drop in folder (and then they won't need to access the filesystem). Once processed/acked, file can be deleted by the node.

@clintongormley
Copy link
Contributor Author

We could maybe use a "drop in" folder where the user can copy the scripts and they will be loaded and added as stored script by the node itself?

The way to do this is, when building your package, to fire up a one node cluster with the right cluster name, add all the scripts/templates/whatever that is needed for the new cluster, then shut the node down and use it as a base image.

Then when deploying, the first node in your cluster has all the right settings etc, and the other nodes that join the cluster inherit them directly from the cluster state (ignoring whatever they have locally).

@bleskes
Copy link
Contributor

bleskes commented Dec 2, 2016

We discussed this further on fix it friday. We are tending to remove file based scripts but want to wait more and get more feedback from users.

Some finer points of the discussion that are worth mentioning:

  1. File base scripts are used today to add scripts while blocking them completely in the APIs. Even if we remove file based scripts people can still use plugins for this case.
  2. The method suggested by @clintongormley works for the initial deployment of a cluster, it does however fail when one want to modify an existing cluster. For example, currently you can shutdown the cluster, update the scripts, start the cluster and have new scripts deployed.

@javanna
Copy link
Member

javanna commented May 5, 2017

Seems like we got no feedback from users in 5 months. Maybe time to mark adoptme?

@clintongormley
Copy link
Contributor Author

I'm in favour of removing file scripts and just keeping stored and inline. The APIs for storing scripts should be protected by the application. Marking as adoptme

@clintongormley clintongormley added help wanted adoptme and removed discuss labels May 8, 2017
rjernst added a commit to rjernst/elasticsearch that referenced this issue May 9, 2017
File scripts have 2 related settings: the path of file scripts, and
whether they can be dynamically reloaded. This commit deprecates those
settings.

relates elastic#21798
rjernst added a commit that referenced this issue May 9, 2017
File scripts have 2 related settings: the path of file scripts, and
whether they can be dynamically reloaded. This commit deprecates those
settings.

relates #21798
rjernst added a commit that referenced this issue May 9, 2017
File scripts have 2 related settings: the path of file scripts, and
whether they can be dynamically reloaded. This commit deprecates those
settings.

relates #21798
rjernst added a commit to rjernst/elasticsearch that referenced this issue May 11, 2017
This commit removes file scripts, which were deprecated in 5.5.

closes elastic#21798
rjernst added a commit that referenced this issue May 17, 2017
This commit removes file scripts, which were deprecated in 5.5.

closes #21798
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache feedback_needed help wanted adoptme
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants