Skip to content

Fix migrations to unblock Kibana devs who are working with ES7 (master)#28369

Merged
chrisdavies merged 1 commit intoelastic:masterfrom
chrisdavies:migrations/support-es-7
Jan 10, 2019
Merged

Fix migrations to unblock Kibana devs who are working with ES7 (master)#28369
chrisdavies merged 1 commit intoelastic:masterfrom
chrisdavies:migrations/support-es-7

Conversation

@chrisdavies
Copy link
Copy Markdown
Contributor

@chrisdavies chrisdavies commented Jan 9, 2019

Fix #28358

ES7 has changed their mappings structure (removing the root-level doc property). That caused migrations to error when Kibana restarts.

This is a quick fix to unblock people who are working on Kibana + ES master. It normalizes the mappings we get from ES6 and ES7.

@chrisdavies chrisdavies requested a review from a team January 9, 2019 16:15
@joshdover
Copy link
Copy Markdown
Contributor

joshdover commented Jan 9, 2019

@chrisdavies Instead of having a normalize function, should we just make the master branch work with the 7.0 API and leave the 6.x branch alone?

I could see an argument that this will make 6.x harder to maintain/backport fixes to, but we also only have one more minor release left in the 6.x series.

I tend to prefer moving the master branch forward to be completely typeless. Thoughts?

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jan 9, 2019

💚 Build Succeeded

@chrisdavies
Copy link
Copy Markdown
Contributor Author

chrisdavies commented Jan 9, 2019

@joshdover I'm down. The problem is that our yarn es snapshot and ElasticSearch master are out of sync. This change allows devs to develop against either. I think that fixing it in a 7.0 only way will break es snapshot, but I can double check.

@chrisdavies
Copy link
Copy Markdown
Contributor Author

chrisdavies commented Jan 9, 2019

OK. Following up. I did the reverse, normalizing on v7's mapping format rather than v6, and this broke things both in es snapshot and ES master. The reason is that much of Kibana assumes a mapping format of doc.properties or whatever. So, I my suggestion is to move forwards with this PR as is (or with tweaks), and come up with a way to migrate all Kibana code over to the new, flatter mapping structure as a separate PR / issue.

joshdover
joshdover approved these changes Jan 10, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@joshdover joshdover left a comment

Choose a reason for hiding this comment

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

I'm good with this fix for the time being. Will definitely need to make sure migrations is fixed for 7.0 as part of #23650

@chrisdavies chrisdavies merged commit 2b593f1 into elastic:master Jan 10, 2019
@chrisdavies chrisdavies deleted the migrations/support-es-7 branch January 10, 2019 15:18
spalger pushed a commit to spalger/kibana that referenced this pull request Jan 10, 2019
Revert "Fix migrations to unblock Kibana devs who are working with ES…
26515e9 
…7 (master) (elastic#28369)"

This reverts commit 2b593f1.
@spalger spalger mentioned this pull request Jan 10, 2019
Merged
spalger pushed a commit that referenced this pull request Jan 10, 2019
[migrations] fetch mappings with types (#28497)
8361b82 
* Revert "Fix migrations to unblock Kibana devs who are working with ES7 (master) (#28369)"

This reverts commit 2b593f1.

* [server/migrations] request mappings with type names

* [server/migrations] remove unnecessarily async function

* update tests to use/check for include_type_name: true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@joshdover joshdover joshdover approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

v7.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Kibana fails to restart on the latest ES master

3 participants

@chrisdavies @joshdover @elasticmachine