[BCI-3988] - FilteredLogs receive []Expression instead of whole KeyFilter #14109
Conversation
Farber98
changed the title
core/services/ocr2/plugins/ccip/internal/ccipdata/v1_2_0/commit_store.go
Outdated
Show resolved
Hide resolved
core/services/ocr2/plugins/ccip/internal/ccipdata/v1_0_0/commit_store.go
Outdated
Show resolved
Hide resolved
|
| assert.Equal(t, []query.Expression{}, result) | ||
| }) | ||
|
|
||
| t.Run("Invalid boolean expression", func(t *testing.T) { |
There was a problem hiding this comment.
are the valid and invalid expressions documented somewhere? Would it make sense to go through all them in these tests and show how to properly use them?
There was a problem hiding this comment.
This logpoller.Where() function it's just a wrapper of the original one query.Where() where you don't need to provide a key param and just the expressions
The underlying function has extensive tests here which uses the test cases defined here. Also the usage eg. defined in the function comment is clear
Let me know your thoughts if this seems enough
| t.Run("Valid combination of filters", func(t *testing.T) { | ||
| result, err := logpoller.Where(expr1, expr2, expr3, expr4) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, []query.Expression{expr1, expr2, expr3, expr4}, result) |
There was a problem hiding this comment.
I assume these filters are being combined as a logical "AND" - the events need to match all filters. What if you want a logical "OR" - events that match at least one of the filters?
There was a problem hiding this comment.
I was testing this function at a high level, maybe it's redundant given the underlying tests. More details in my previous comment here: #14109 (comment)
Mateusz commented here the following:
We only need to merge core and they will eventually get these changes. We only cherry pick when changes needed are urgent as Makram said here too |
patrick-dowell
deleted the
BCI-3988-remove-keyfilter-from-filteredlogs
branch
Task Description:
Log poller Filtered logs have no use for Key, which makes the method confusing when used outside of CR
This PR:
FilteredLogsmethod so it receives[]query.Expressioninstead of wholequery.KeyFilteras param.logpoller.Wherefunction that doesn't receive key. It wrapsquery.WhereTicket:
Note:
[BCI-3863] - Use filtered logs in eventBinding GetLatestValue instead of manual filtering #14096 will introduce small conflicts as it adds a new reference tofixedFilteredLogsMerged with:
[BCI-3988] - FilteredLogs receive []Expression instead of whole KeyFilter ccip#1403not needed based on Mateusz comment within the PR