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

[Detection Engine] update lists API #4067

Merged
merged 15 commits into from
Nov 3, 2023

Conversation

vitaliidm
Copy link
Contributor

@vitaliidm vitaliidm commented Oct 18, 2023

Fixes #4069

updates list API description with data stream mentions

@github-actions
Copy link

Documentation previews:

@vitaliidm vitaliidm marked this pull request as draft October 25, 2023 09:05
@vitaliidm vitaliidm marked this pull request as ready for review October 25, 2023 14:15
Copy link
Contributor

@natasha-moore-elastic natasha-moore-elastic left a comment

Choose a reason for hiding this comment

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

Thanks again for making these changes, @vitaliidm! Looks good overall, just a few questions (GH won't let me comment on the specific lines):

  1. In lists-index-api-overview.asciidoc, should we update the following ’index’ / ‘indices’ references?

    • line 40: "Get index"
    • line 81: "Example response when the indices do not exist"
  2. In detections-req.asciidoc, should we add “and data streams” on the following lines?

    • line 74: "The manage, write,read, and view_index_metadata index privileges for the following system indices"
    • line 99: "The manage, write,read, and view_index_metadata index privileges for the following system indices, where <space-id> is the {kib} space name"
    • line 121: "The maintenance, write,read, and view_index_metadata index privileges for the following system indices, where <space-id> is the {kib} space name"
  3. If the endpoint itself is still api/lists/index, I’m wondering if we should keep the lists-index-api-overview.html page title as Lists index endpoint, to avoid confusion?

@vitaliidm
Copy link
Contributor Author

@natasha-moore-elastic
Thanks for catching missing items.

I have updated first 2 points.
But don't have strong opinion on the third one - once we mentioned everywhere that .lists and .items are data streams, we probably can change that title.

@nastasha-solomon
Copy link
Contributor

nastasha-solomon commented Oct 25, 2023

If the endpoint itself is still api/lists/index, I’m wondering if we should keep the lists-index-api-overview.html page title as Lists index endpoint, to avoid confusion?

I agree, @natasha-moore-elastic. Another reason to keep the title as it is:
My very rudimentary understanding of data streams, is that they're made up of indices. If this is the case, the current title (Lists index endpoint) is technically correct.

Also, I'm not sure where this UI text is stored, but I think we also need to update the banner that appears on the Alerts page if users don't have certain privs. This is what I'm talking about, @vitaliidm:

Screenshot 2023-10-25 at 3 31 43 PM

@vitaliidm
Copy link
Contributor Author

vitaliidm commented Oct 26, 2023

@nastasha-solomon , changed back list index endpoint

Here is draft PR: elastic/kibana#169916
It worth to note, it is still missing index priveleges, but for data streams. I hope it won't bring any additional confusion to users. So, might consider leaving it as index for .lists/.items? Or shall we move with data stream? Please review that PR

@nastasha-solomon
Copy link
Contributor

nastasha-solomon commented Oct 26, 2023

@vitaliidm can you elaborate on this:

It worth to note, it is still missing index priveleges, but for data streams.

I'm mainly suggesting that we update the text in the Insufficient privileges banner so that it aligns with the changes you made here.

@nastasha-solomon nastasha-solomon self-requested a review October 27, 2023 13:00
Copy link
Contributor

@nastasha-solomon nastasha-solomon left a comment

Choose a reason for hiding this comment

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

Looks good to go!

@nastasha-solomon nastasha-solomon mentioned this pull request Nov 2, 2023
15 tasks
@natasha-moore-elastic natasha-moore-elastic merged commit 7db2b5b into elastic:main Nov 3, 2023
1 check passed
mergify bot pushed a commit that referenced this pull request Nov 3, 2023
* update lists API

* fix typo

* updates

* more changes

* updates

* remove /index mention

* CR

* Update lists-index-api-overview.asciidoc

---------

Co-authored-by: Nastasha Solomon <[email protected]>
Co-authored-by: natasha-moore-elastic <[email protected]>
(cherry picked from commit 7db2b5b)
nastasha-solomon added a commit that referenced this pull request Nov 6, 2023
Co-authored-by: Nastasha Solomon <[email protected]>
Co-authored-by: natasha-moore-elastic <[email protected]>
Co-authored-by: Vitalii Dmyterko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List indices being moved to data streams
3 participants