-
Notifications
You must be signed in to change notification settings - Fork 605
[FR] [DAC] Add index_or_dataview Property #3857
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
[FR] [DAC] Add index_or_dataview Property #3857
Conversation
|
@eric-forte-elastic any reason why we can't do this in #3830 ? |
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 work 👍🏽
- Added a comment about ES|QL rules having no index nor dataview
- Checked other
...RuleDataclasses that inherityQueryRuleDataand they seem fine with these updates - Did not review for small nits
As always, I'd run a build package with the changes before merging to ensure that packaging, rule validation and schema's are all good. Approving not to block.
Looking at the implementation, I think we could do this in 3830 (originally there was some concern that we might not be able to) However, since we have most the implementation discussion in this issue and we expect that we may merge to main fairly shortly, I think it is best to keep them separate. |
| """Return the index or dataview depending on which is set. If neither returns empty list.""" | ||
| if self.index is not None: | ||
| return self.index | ||
| elif self.dataview is not None: |
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.
This should be self.data_view_id.
Issues
#3697
Summary
This PR adds a property to
QueryRuleDatathat allows you to use either index or data view depending on which one is set. This PR is dependent on a validation update to main that prevents both indexes and data views from being set.Please review and merge #3830 first and backport this to DAC-Feature before reviewing this PR. Update: 3830 is now Merged.
Note: There are a few remaining
data.indexcalls that do not appear to support using this property. For example, invalidate_eqlthis update is not supported asdata.indexis used as an index parameter to theself.es_client.eql.searchfunction call which does not support data views.Testing
Make a package using
maketo make sure the new property fills the requirements of the updated function calls that used to use.index.