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

Deprecate Empty Templates #30194

Merged
merged 5 commits into from
May 14, 2018
Merged

Deprecate Empty Templates #30194

merged 5 commits into from
May 14, 2018

Conversation

jdconrad
Copy link
Contributor

Deprecates empty templates. This will be followed up with another PR for just 7.0 that will remove the ability to add empty templates, and will drop empty templates during upgrades with a warning.

Also removes a backwards incompatible check for empty code strings being loaded from stored scripts. Search templates did not have an appropriate check for not allowing stored scripts to have an empty code string, so when an empty search template is added, future restarts of the node will fail.

@jdconrad jdconrad requested a review from martijnvg April 26, 2018 23:20
@jdconrad jdconrad added the :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache label Apr 26, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jdconrad
Copy link
Contributor Author

jdconrad commented May 7, 2018

@martijnvg Hey Martijn, if you're back from holiday, would appreciate you having a look at this when you have a spare moment. Thanks!

@nik9000
Copy link
Member

nik9000 commented May 7, 2018

5.6.10 seems like a long way to backport this. If you plan from removing from master I think it is fine to release this with 6.4.

*/
private StoredScriptSource build() {
private StoredScriptSource build(boolean ignore) {
Copy link
Member

Choose a reason for hiding this comment

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

s/ignore/ignoreEmpty/ maybe?

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@martijnvg
Copy link
Member

I agree with @nik9000 comment about just getting this change in 6.x branch and master.

@jdconrad
Copy link
Contributor Author

@martijnvg @nik9000 Thanks for the reviews! Will push as soon as CI is completed.

@jdconrad jdconrad merged commit 1b0e6ee into elastic:master May 14, 2018
jdconrad added a commit that referenced this pull request May 16, 2018
Deprecate the use of empty templates.  Bug fix allows empty
templates/scripts to be loaded on start up for upgrades/restarts, 
but empty templates can no longer be created.
martijnvg added a commit that referenced this pull request May 17, 2018
* es/6.x: (44 commits)
  SQL: Remove dependency for server's version from JDBC driver (#30631)
  Make xpack modules instead of a meta plugin (#30589)
  Security: Remove SecurityLifecycleService (#30526)
  Build: Add task interdependencies for ssl configuration (#30633)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (#30646)
  Reindex: Fixed typo in assertion failure message (#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  Use readFully() to read bytes from CipherInputStream (#30640)
  Add Create Repository High Level REST API (#30501)
  [DOCS] Reorganizes RBAC documentation
  Test: increase search logging for LicensingTests
  Delay _uid field data deprecation warning (#30651)
  Deprecate Empty Templates (#30194)
  Remove unused DirectoryUtils class. (#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (#30534)
  [TEST] Remove AwaitsFix in IndicesOptionsTests#testSerialization
  S3 repo plugin populates SettingsFilter (#30652)
  Rest High Level client: Add List Tasks (#29546)
  Fixes IndiceOptionsTests to serialise correctly (#30644)
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants