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

CORE: Allow Setting Index Autocreate to _template #32964

Closed
wants to merge 21 commits into from

Conversation

original-brownbear
Copy link
Member

@original-brownbear original-brownbear commented Aug 17, 2018


  • Marked WIP because I have some open questions here, that I will ask below in a bit.
  • Also, solution for watcher compatibility is missing yet, incoming though

* Enables setting autocreate by template as described in elastic#20640
* Closes elastic#20640
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

String defaultTemplateName = "_default";
upgradedTemplates.computeIfAbsent(
defaultTemplateName,
def -> IndexTemplateMetaData.builder(defaultTemplateName).autoCreateIndex(true)
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this a reasonable default?

builder.patterns(in.readList(StreamInput::readString));
List<String> indexPatterns = in.readList(StreamInput::readString);
if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
builder.autoCreateIndex(Boolean.parseBoolean(indexPatterns.remove(indexPatterns.size() - 1)));
Copy link
Member Author

Choose a reason for hiding this comment

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

Adding a serialized boolean to the end of this list feels really dirty, but I couldn't get the BwC tests to pass when adding a new byte to the message (even when wrapping writing that boolean in a if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {). Maybe I'm making a mistake here?

if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {`
 autoCreateIndex = in.readBoolean();
}

did not work for me in BwC at least.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe this can be resolved in a different way since the issues mentions users will have to update their templates when using the new auto create mode and BwC isn't really an issue?

@original-brownbear
Copy link
Member Author

original-brownbear commented Aug 21, 2018

@jasontedor can you take a quick look here whenever you have a sec?

I'm mainly looking for confirmation that this is going in the right direction functionally and for answers for my two questions (the default + the serialization BwC).

If this is what we want, then I think only integration tests + fixing the watcher compatibility are left (Watcher validates that the auto create pattern allows for its indexes to be created, which fails if the setting is just _template, so that validation will need some additional logic).

@original-brownbear
Copy link
Member Author

@jasontedor ping (if you have a sec :))

@jasontedor jasontedor requested review from nik9000 and removed request for jasontedor September 8, 2018 02:43
@nik9000
Copy link
Member

nik9000 commented Oct 2, 2018

I think this should wait until we have worked out #21105.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >feature WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace action.auto_create_index with a template setting
8 participants