-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Make task manager index configurable again #42394
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
Conversation
|
Pinging @elastic/kibana-stack-services |
This comment has been minimized.
This comment has been minimized.
1869b3d to
a2b4fe5
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
eliperelman
left a comment
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.
LGTM
| convertToAliasScript: `ctx._id = ctx._source.type + ':' + ctx._id`, | ||
| indexPattern(config) { | ||
| return config.get('xpack.task_manager.index'); | ||
| }, |
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.
Ah I was expecting the entire savedObjectSchema to be a function to be more similar to other uiExports. If you're confident there's no use case for that, then this is fine.
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.
Yeah I can't think of any yet. I believe if we make a function at the savedObjectSchema level and use reducers, I don't think the config object is available at that time. But so far this is the only configurable property I can think of.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
💔 Build Failed |
bmcconaghy
left a comment
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.
Code LGTM
bmcconaghy
left a comment
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.
Code LGTM
💚 Build Succeeded |
pmuellr
left a comment
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.
LGTM
* Initial work * Fix type check * Accept core API changes * Fix broken tests * Destructure index pattern
In this PR, I'm making the task manager index configurable again. This was changed to static in this PR #39829 and will be configurable again once this PR merges.
Some of the core changes required:
indexPatternin savedObjectsSchemasstring | (config) => stringinstead of only being a string. This allows to pull values from the server config.createIndexMapconstructor args a single destructured argumentcreateIndexMaptake an instance ofSavedObjectsSchemainstead of the rawsavedObjectsSchemasto avoid code duplication.