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

event: Extract DispatcherBase interface #14446

Merged
merged 3 commits into from
Dec 17, 2020

Conversation

antoniovicente
Copy link
Contributor

Commit Message:
event: Extract DispatcherBase interface

This new interface allows us to more safely build primitives like ScaledTimers which are themselves built on top of low-level
event loop primitives like Timers which are themselves created via the dispatcher interface.
Additional Description:
Risk Level: n/a interface only change
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Issue #14401

Signed-off-by: Antonio Vicente <avd@google.com>
Signed-off-by: Antonio Vicente <avd@google.com>
@antoniovicente antoniovicente marked this pull request as ready for review December 16, 2020 20:07
akonradi
akonradi previously approved these changes Dec 16, 2020
Copy link
Contributor

@akonradi akonradi left a comment

Choose a reason for hiding this comment

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

Looks good. Have you tried refactoring any of the impl classes returned by the dispatcher to use DispatcherBase yet?

@@ -52,12 +52,67 @@ using PostCb = std::function<void()>;
using PostCbSharedPtr = std::shared_ptr<PostCb>;

/**
* Abstract event dispatching loop.
* Minimal interface to the disptching loop used to create low-level primitives. See Dispatcher
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: sp on dispatching is causing your format failure

@mattklein123
Copy link
Member

This makes sense to me but this is small enough that do we want to just do this change in another change that actually needs it? Up to you all.

/wait

Signed-off-by: Antonio Vicente <avd@google.com>
@antoniovicente
Copy link
Contributor Author

This makes sense to me but this is small enough that do we want to just do this change in another change that actually needs it? Up to you all.

/wait

@akonradi Thoughts? Do you want to include this in a PR that adds a way to create scaled timers via the dispatcher?

@akonradi
Copy link
Contributor

Given that this is already approved, I say let's get it in now. No need to make the next PR bigger.

@antoniovicente
Copy link
Contributor Author

Thanks Matt.

@antoniovicente antoniovicente merged commit e630672 into envoyproxy:master Dec 17, 2020
itamarkam pushed a commit to itamarkam/envoy that referenced this pull request Dec 21, 2020
Signed-off-by: Antonio Vicente <avd@google.com>
itamarkam pushed a commit to itamarkam/envoy that referenced this pull request Jan 25, 2021
Signed-off-by: Antonio Vicente <avd@google.com>
itamarkam added a commit to itamarkam/envoy that referenced this pull request Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants