Skip to content

Conversation

@agrawroh
Copy link
Member

Description

This PR adds timer API to the Dynamic Modules network filter ABI.


Commit Message: dynamic_modules: add timer to network filters ABI
Additional Description: Added timer API to the Dynamic Modules network filter ABI.
Risk Level: Low
Testing: Added Tests
Docs Changes: Added
Release Notes: N/A

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #42957 was opened by agrawroh.

see: more, trace.

@agrawroh agrawroh marked this pull request as ready for review January 12, 2026 22:29
Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. I am against adding a timer ABI. Users can use scheduler ABI to achieve this and that's why HTTP filter doesn't have this one

@agrawroh
Copy link
Member Author

hmm. I am against adding a timer ABI. Users can use scheduler ABI to achieve this and that's why HTTP filter doesn't have this one

Hmmm, do we think the scheduler is enough?

@mathetake
Copy link
Member

I believe so but i would love the input from @wbpcode too.

@wbpcode
Copy link
Member

wbpcode commented Jan 13, 2026

I agree with @mathetake in some way 🤔 Why this is necessary and which scenarios are not supported by current way?

@agrawroh
Copy link
Member Author

I agree with @mathetake in some way 🤔 Why this is necessary and which scenarios are not supported by current way?

My understanding is that scheduler is not cancellable and can't be re-armed with a different duration. Also, timers are more of "do this later" vs. scheduler which is "do this now from another thread".

We could have use-cases that actually benefit from having timers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants