-
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
[Security Solution][Data Views] - Update bulk actions data views index param to be snake_case #138304
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
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.
Great catch @yctercero, we overlooked that too.
The diff LGTM. I also tested it locally and it works 👍
Except for the failing integration test, this should be ready for merge.
One question: I didn't find this overwrite_data_views
or overwriteDataViews
request parameter documented in https://www.elastic.co/guide/en/security/master/bulk-actions-rules-api.html. Do we have a ticket in https://github.com/elastic/security-docs for documenting it? Do you think it's ready to be documented?
Btw I don't know if you've seen the update but the |
Thanks @banderror ! I'm updating the integration test now. As far as documenting, it is being worked on here by the docs team as part of the new data views in rule creation work. |
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.
Awesome, thank you for the info @yctercero! 🚀
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @yctercero |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…x param to be snake_case (elastic#138304) ## Summary Overlooked a param name - should be snake case per our APIs. `overwriteDataViews` --> `overwrite_data_views` (cherry picked from commit bad17a1)
Hey @yctercero I think you needed to rebase against |
…atterns (elastic#138435) **Ticket:** elastic#138409 **Caused by:** elastic#138304 (comment) ## Summary The merge of elastic#138304 caused unit tests for bulk editing index patterns to start failing. This PR was reverted by elastic@0bc8cf7. The tests were skipped in elastic@26a4783. These tests are fully functional in `main`. Unskipping them.
Summary
Overlooked a param name - should be snake case per our APIs.
overwriteDataViews
-->overwrite_data_views
Testing
Checklist
For maintainers