-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Surface data streams in Index Management. #67806
Surface data streams in Index Management. #67806
Conversation
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
@alisonelizabeth Note that we're overlapping in terms of adding the Elasticsearch client custom endpoints. So I guess whoever merges last will resolve the merge conflicts? I don't think they'll be tough to resolve. |
@cjcenizal I was planning on merging the component templates server side code to master, as @sebelga also is dependent on some of these changes. Let me know if you have any concerns. Related PR: #66596 |
@cjcenizal I am not seeing the Is this known? |
c4e885e
to
72d3fb7
Compare
- Add GET data streams API endpoint. - Render table of data streams in its own tab.
…be more fully featured than the Data Streams tab and this order will be less of a shock to users who upgrade.
…eam and deleteDataStream methods.
72d3fb7
to
472badd
Compare
472badd
to
9e070c0
Compare
…abs. - Refactor routing service to expose decodePathFromReactRouter and encodePathForReactRouter methods to clarify their purpose. - Add stub data stream detail panel, with the intention to populate it with content in the future. - Converted index_list to TS.
9e070c0
to
fdd85ad
Compare
… Data Streams tabs. Fix TS error with API integration test.
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.
Nice work @cjcenizal! Left a few comments but nothing blocking. Tested locally and everything worked as expected.
import { indexManagementStore } from '../../../public/application/store'; // eslint-disable-line @kbn/eslint/no-restricted-paths | ||
import { WithAppDependencies, services, TestSubjects } from '../helpers'; | ||
|
||
const testBedConfig: TestBedConfig = { | ||
store: () => indexManagementStore(services as any), | ||
memoryRouter: { | ||
initialEntries: [`/indices?includeHiddenIndices=true`], | ||
componentRoutePath: `/:section(indices|templates)`, | ||
componentRoutePath: `/:section(indices|data_streams)`, |
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.
Should this be indices|data_streams|templates
?
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 think we'd need to add templates
if these tests ever navigate to that tab, but because they don't I think we should leave it out.
store: () => indexManagementStore(services as any), | ||
memoryRouter: { | ||
initialEntries: [`/indices`], | ||
componentRoutePath: `/:section(indices|data_streams)`, |
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.
Should this be indices|data_streams|templates
?
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.
Same reasoning here. WDYT?
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.
Though now that I've added the link to index templates from the empty prompt, I see a reason to add it here. :)
const createDataStream = (name: string) => { | ||
// A data stream requires an index template before it can be created. | ||
return supertest | ||
.post(`${API_BASE_PATH}/index-templates`) |
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.
Do you think it would be better to using the legacyEs
to create the index template directly, instead of calling our endpoint?
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 added it to the elasticsearch client via 78f939b (PR still in review).
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.
Good call! Done.
|
||
describe('Data streams', function () { | ||
// TODO: Implement this test once the API supports creating composable index templates. | ||
describe.skip('Get', () => { |
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.
Does this still need to be skipped (see comment above)?
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.
Unskipped.
onClose: () => void; | ||
} | ||
|
||
export const DataStreamDetailPanel: React.FunctionComponent<Props> = ({ |
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.
Do you think it's worth adding a comment to this file that it's not currently being used and is a WIP?
<EuiEmptyPrompt | ||
iconType="managementApp" | ||
title={ | ||
<h1 data-test-subj="title"> |
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.
[nit] I think we've typically added a description to the empty prompt as well.
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.
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.
Integration with indices tab via URL looks good to me! Nice work @cjcenizal !
@@ -39,3 +33,8 @@ export const decodePath = (pathname: string): string => { | |||
} | |||
return decodeURIComponent(decodedPath); | |||
}; | |||
|
|||
// Need to add some additonal encoding/decoding logic to work with React Router | |||
// For background, see: https://github.com/ReactTraining/history/issues/505 |
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.
Thanks for adding this!
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.
To be fair I think the comment was already there! I just moved it around. :)
- Add link to Index Templates from Data Streams empty prompt and component integration test. - Extend legacy ES client with index templates API and unskip API integration test. - Add comment to unused detail panel file.
# Conflicts: # x-pack/plugins/index_management/public/application/services/api.ts
* master: (22 commits) Partial revert of "Sync Kerberos + Anonymous access tests with the latest `security/_authenticate` API (user roles now include roles of anonymous user)." (elastic#68624) adapt some snapshot test (elastic#68489) [APM] Service maps - Fix missing ML status for services with jobs but no anomalies (elastic#68486) [skip test] apis Kerberos security Kerberos authentication finishing SPNEGO should properly set cookie and authenticate user [SIEM][Exceptions] - ExceptionsViewer UI component part 2 (elastic#68294) Surface data streams in Index Management. (elastic#67806) Fix edit datasource not working following changes in elastic#67234 (elastic#68583) [Logs + Metrics UI] Clean up async plugin initialization (elastic#67654) APM Storybook fixes (elastic#68671) Upgrade EUI to v24.1.0 (elastic#68141) [ML] DF Analytics: Creation wizard part 2 (elastic#68462) [Uptime] Fix race on overview page query (elastic#67843) Prefer using npm_execpath when spawning Yarn (elastic#68673) [Security] [Cases] Attach timeline to existing case (elastic#68580) Use Search API in Vega (elastic#68257) [Component templates] Table view (elastic#68031) [Uptime] Added relative date info in cert status column (elastic#67612) [Endpoint] Re-enable Functional test case for Endpoint related pages (elastic#68445) run page_load_metrics tests in visual regresssion jobs (elastic#68570) Enable exhaustive-deps; correct any lint warnings (elastic#68453) ...
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/index_management/home_page·ts.Index Management app Home page Component templates renders the component templates tabStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/doc_level_security_roles·js.security app dls user East should only see EAST docStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/security/doc_level_security_roles·js.security app dls user East should only see EAST docStandard Out
Stack Trace
and 4 more failures, only showing the first 3. History
To update your PR or re-run it, just comment with: |
Changes
decodePathFromReactRouter
andencodePathForReactRouter
methods, to make their purpose more obvious.Testing
To test, create some data streams in Console and see them in Index Management.
Release note
We added a "Data Streams" tab to Index Management to help users manage their data streams.