Skip to content

dispatcher: split into multiple interfaces to improve functional scoping#16709

Merged
junr03 merged 6 commits intoenvoyproxy:mainfrom
goaway:ms/dispatcher-interfaces
Jun 11, 2021
Merged

dispatcher: split into multiple interfaces to improve functional scoping#16709
junr03 merged 6 commits intoenvoyproxy:mainfrom
goaway:ms/dispatcher-interfaces

Conversation

@goaway
Copy link
Copy Markdown
Contributor

@goaway goaway commented May 27, 2021

Commit Message: dispatcher: split into multiple interfaces to improve functional scoping
Additional Description: The Envoy event dispatcher is a bit of a multi-tool, and often only a small portion of its interface is used in a given context. Envoy Mobile has various cases where it needs to wrap or extend pieces of the Event::Dispatcher, but it's difficult to simply wrap it in its entirety given the amount of functionality it has. Having it implement more narrowly scoped interfaces makes it easier to do this.

This can also simplify mocking and testing dispatcher functionality in different contexts and could eventually support more compartmentalized implementation as well.
Risk Level: Moderate
Testing: Local & CI

goaway added 2 commits May 28, 2021 05:31
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
@goaway goaway changed the title Ms/dispatcher interfaces dispatcher: split into multiple interfaces to improve functional scoping May 27, 2021
@goaway
Copy link
Copy Markdown
Contributor Author

goaway commented May 27, 2021

Note this is straight refactor; it only moves the function declarations between interfaces, all of which the Dispatcher still implements. I did a search for the previous DispatcherBase and didn't find any usage.

Signed-off-by: Mike Schore <mike.schore@gmail.com>
@phlax phlax assigned antoniovicente and junr03 and unassigned antoniovicente May 28, 2021
@antoniovicente antoniovicente self-assigned this Jun 1, 2021
goaway added 2 commits June 3, 2021 19:34
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
@goaway
Copy link
Copy Markdown
Contributor Author

goaway commented Jun 4, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16709 (comment) was created by @goaway.

see: more, trace.

@junr03
Copy link
Copy Markdown
Member

junr03 commented Jun 4, 2021

@antoniovicente did you have additional feedback here?

@goaway do you mind merging main?

Signed-off-by: Mike Schore <mike.schore@gmail.com>
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente 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.

Sorry for the delays in review. I have been moving and that has required being OOO for a few days.

@antoniovicente
Copy link
Copy Markdown
Contributor

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16709 (comment) was created by @antoniovicente.

see: more, trace.

@antoniovicente
Copy link
Copy Markdown
Contributor

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16709 (comment) was created by @antoniovicente.

see: more, trace.

@goaway
Copy link
Copy Markdown
Contributor Author

goaway commented Jun 9, 2021

Thanks @antoniovicente!

@goaway
Copy link
Copy Markdown
Contributor Author

goaway commented Jun 9, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16709 (comment) was created by @goaway.

see: more, trace.

@junr03 junr03 merged commit fc5c201 into envoyproxy:main Jun 11, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
…ing (envoyproxy#16709)

Commit Message: dispatcher: split into multiple interfaces to improve functional scoping
Risk Level: Moderate
Testing: Local & CI

Signed-off-by: Mike Schore <mike.schore@gmail.com>
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