-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Import saved searches first #10740
Import saved searches first #10740
Conversation
}); | ||
} | ||
|
||
const docTypes = docs.reduce((types, doc) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about something like this?
const [searches, other] = _.partition(docs, doc => doc._type === "search")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slick, but that only allows me to create 2 collections. That works for now, but I'm planning to add more exports, and order will become important in those cases too. This structure makes it easy to add more ordered collections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_.groupBy(docs, doc => {
switch (doc._type) {
case 'search':
return 'searches'
default:
return 'other'
}
})
Functionality LGTM. Reviewing code now... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like @kjbekkelund's suggestion to use _.groupBy
. This code is trying to group objects by their type and _.groupBy
conveys that more explicitly than docs.reduce
.
If you are trying to avoid using lodash, then I'd suggest creating a function named groupDocsByType
or something that encapsulates your code. That way, its slightly more obvious what's going on.
Additionally, |
For me, its less about whether we use a native method or a lodash method and more about descriptiveness of the operation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality still LGTM and code LGTM as well.
LGTM. One note, since this relies on |
@Bargs just seeing your comment now, sorry. Isn't that going to be a 7.0 concern? Either way, yeah, it's part of a larger change. This at least makes things work better right now. |
It'll be a 6.0 thing because new indices will only be able to have one type in 6.0 elastic/elasticsearch#15613 (comment) |
* import saved searches first * move saved object type grouping into a helper
* import saved searches first * move saved object type grouping into a helper
* import saved searches first * move saved object type grouping into a helper
* import saved searches first * move saved object type grouping into a helper
5.x ba92d47 |
* import saved searches first * move saved object type grouping into a helper
Closes #7667
When importing saved objects from JSON, those objects are run through the saved object service, which means that visualizations and searches check their fields and other hierarchical requirements. For visualizations, this means that any associated saved searches are looked up on import, and things fail if the required saved search does not exist.
However, all saved objects were imported in parallel, which led to weird race conditions where visualizations would fail to import because their saved searches were not yet finished importing.
With this PR, saved searches are imported before anything else, so this race condition is no longer an issue.
To test, either:
If you are interested in the later, here's a handy diff: