[Merged by Bors] - feat: added look_back to smartmodule proc macro
#3276
Conversation
|
LGTM! One nitpick, is there a recommended error for user look_back logic to use and attach "application" level context to? A ui-test where user code explicitly returns an error in the lookback fn might be good. |
Added an example in ui-tests here 310f6a7 |
| transforms: | ||
| - uses: infinyon/fluvio-smartmodule-filter-lookback@0.1.0 | ||
| with: | ||
| lookback: |
There was a problem hiding this comment.
There is no loopback in the code example. It should be removed. The loopback will be controlled by SPU
There was a problem hiding this comment.
Not part of this issue but we should have ability to test loopback using SMDK
There was a problem hiding this comment.
There is no
loopbackin the code example. It should be removed. The loopback will be controlled by SPU
This parameter is consumed on SmartEngine and SPU levels. It will come in the next PRs.
The loopback is controlled by SPU, that's correct. But it still needs to get information of how many records need to be read and passed to the SM via look_back call, and that is where this parameter comes to play.
There was a problem hiding this comment.
Not part of this issue but we should have ability to test loopback using SMDK
Good point. Added it to the tracking issue
There was a problem hiding this comment.
Can't rely on SmartModule on having parameters since it is dependent on author. This need to be assume be defined by SPU
There was a problem hiding this comment.
It is supposed to be defined not by the author but by the user of the SmartModule. Some SmartModules need only the last record (github stars, deduplication by monotonic ids, etc), some SmartModules need to read 1000 last records to build some data structure (bloom filters). Later, we may want to introduce asking for records based on time, etc. This is a concern of the configuration of the SmartModule. SPU has no this information.
There was a problem hiding this comment.
This is how it was discussed in RFD https://github.com/infinyon/infinyon-rfd/pull/23 and already implemented
There was a problem hiding this comment.
Not showstopper but what happens in if there are other filters and transforms in chain. They may have their own lookback param. So maybe lookback should be associated with chain rather than individual transform steps
In that case transform may more look like this
transforms:
lookback:
last: 10
steps:
- mytranform
- transform2
- bloom3
They will get records according to their own lookback parameter. |
|
bors r+ |
Added `#[smartmodule(look_back)]` proc macro to support generating `look_back` function in the SmartModule. Fixes #3273
|
Pull request successfully merged into master. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
bors
bot
changed the title
look_back to smartmodule proc macrolook_back to smartmodule proc macro
Added
#[smartmodule(look_back)]proc macro to support generatinglook_backfunction in the SmartModule.Fixes #3273