[backend] Live stream element dependency containers resolution fetching improvement (#6374) #6375
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6375 +/- ##
==========================================
+ Coverage 66.74% 66.76% +0.01%
==========================================
Files 541 541
Lines 64553 64542 -11
Branches 5306 5307 +1
==========================================
+ Hits 43088 43091 +3
+ Misses 21465 21451 -14 ☔ View full report in Codecov by Sentry. |
| orderBy: 'updated_at', // Get container by newly updated | ||
| orderMode: 'desc', | ||
| connectionFormat: false, | ||
| filters: { |
There was a problem hiding this comment.
couldn't we add streamFilters here to query only elements filtered by this stream ? we wouldn't even need to read the results, only count.
There was a problem hiding this comment.
We could and we should. Just commit a new approach following this remark. Thanks
| if (after || before) { | ||
| const filtersContent = []; | ||
| if (after || before || extraFilters.length > 0) { | ||
| const filtersContent = [...extraFilters]; |
There was a problem hiding this comment.
I'm a bit confused about this, how would it work with after and before ? I know in this current case extraFilters is not used with after or before, but what if it's used in the future ? what filter would it create ? would it be a "And" or "Or" condition between them ?
There was a problem hiding this comment.
For now its a AND condition that feat the needs i needs. Don't want to complexify the options before having a real use case.
There was a problem hiding this comment.
so it's an AND by default in filters ? I just find the code less easy to read when having an if condition between after before and extraFilters, since they are not related (after / before are related). I'm wondering if we really need this extraFilters param, couldn't we create the right FilterGroup object before calling this method ?
There was a problem hiding this comment.
Before and after are not directly related, thats extra options that can accumulate a first set of filters. Its a AND because these filters are use with
mode: FilterMode.And,
filters: filtersContent,
ExtraFilters options is the simplest way to add some filters at the firt level to accumulate with other options.
richard-julien
added
the
solved
Kedae
removed
the
solved
See #6374