Skip to content
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

feat: event arg filtering #55

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

feat: event arg filtering #55

wants to merge 4 commits into from

Conversation

mikeshultz
Copy link
Contributor

@mikeshultz mikeshultz commented Feb 22, 2024

What I did

Adds event filtering by event argument. For example:

@app.on_(USDC.Transfer, start_block=18588777, to="0x0000000000000000000000000000000000000000")
def handle_burn(log):
    return {"burned": log.value}

Will only handle USDC burn events.

fixes: #54
fixes: SBK-417

How I did it

Expands taskiq task definitions to be input arg & value specific by hashing the inputs and using it as a name suffix. When an event is received, it's checked against known event handlers and their filters (if they exist) before sending them along.

How to verify it

Run the example app. Burn events are not uncommon.

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

@mikeshultz mikeshultz added the enhancement New feature or request label Feb 22, 2024
@mikeshultz mikeshultz self-assigned this Feb 22, 2024
@mikeshultz mikeshultz marked this pull request as ready for review February 22, 2024 21:21
example.py Outdated Show resolved Hide resolved
silverback/application.py Outdated Show resolved Hide resolved
@fubuloubu
Copy link
Member

Tested this and got pretty good results!

Add some test cases, and this is probably close to merge

@mikeshultz
Copy link
Contributor Author

Add some test cases, and this is probably close to merge

There are no tests yet in this repo. Perhaps that should be a separate project?

@fubuloubu
Copy link
Member

One issue noted during testing: if you have multiple tasks against the same event/filter arg combo (same named filter arg but different filter values) the second one does not appear to function

This can be potentially resolved in another PR (breaking change) that should make it so there can be multiple handlers defined for a specific subscription (will make this an issue but the previous problem should be solved in some way here)

@fubuloubu
Copy link
Member

fubuloubu commented Apr 8, 2024

PR #66 resolves the "no duplicate handlers" situation, which makes it easier to solve the issue I noted of missing task registrations

@fubuloubu fubuloubu mentioned this pull request Apr 8, 2024
2 tasks
@fubuloubu
Copy link
Member

fubuloubu commented May 31, 2024

Rebased and made updates to use new stateless TaskData setup

TODO:

  • figure out why it keeps failing to start

@fubuloubu fubuloubu marked this pull request as draft June 1, 2024 19:54
Copy link

linear bot commented Jul 10, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow filtering for Event Log tasks
2 participants