Skip to content

Revert "listener filter: hide envoy internals from ListenerFilterCallbacks interface (#22732)#23470

Closed
soulxu wants to merge 1 commit intoenvoyproxy:mainfrom
soulxu:revert_listener_filter_callback_change
Closed

Revert "listener filter: hide envoy internals from ListenerFilterCallbacks interface (#22732)#23470
soulxu wants to merge 1 commit intoenvoyproxy:mainfrom
soulxu:revert_listener_filter_callback_change

Conversation

@soulxu
Copy link
Copy Markdown
Member

@soulxu soulxu commented Oct 13, 2022

Signed-off-by: He Jie Xu hejie.xu@intel.com

Commit Message: Revert "listener filter: hide envoy internals from ListenerFilterCallbacks interface (#22732)
Additional Description:
https://envoyproxy.slack.com/archives/C78HA81DH/p1665661946626769 there is a case showing up, people still need a way to trigger the listener filter iteration in the listener filter.

This reverts commit 350c7f9.

Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

…backs interface (envoyproxy#22732)"

This reverts commit 350c7f9.

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Oct 13, 2022

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Oct 13, 2022

/wait

need to ensure that case is valid first.

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Oct 14, 2022

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #23470 (comment) was created by @soulxu.

see: more, trace.

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Oct 14, 2022

Or I should just expose the continueFilterChain again instead of revert this PR if the dispatch() consider as not expose the listener filter.

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Oct 17, 2022

@mattklein123 looking for feedback on this. This is removed in 1.24 release, if we think it is a valid case, we can add it back before the release. I can also submit a PR and only add that method back since the original PR did some cleanup and refactoring. But if we rush the time, it is also ok to revert this PR.

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Oct 17, 2022

cc @phlax also, in case we want to bring this in before the release

@phlax phlax added this to the 1.24.0 milestone Oct 17, 2022
@mattklein123 mattklein123 self-assigned this Oct 17, 2022
@phlax
Copy link
Copy Markdown
Member

phlax commented Oct 18, 2022

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #23470 (comment) was created by @phlax.

see: more, trace.

@alyssawilk alyssawilk removed this from the 1.24.0 milestone Oct 18, 2022
@KBaichoo
Copy link
Copy Markdown
Contributor

KBaichoo commented Nov 3, 2022

@soulxu I think given this wasn't release blocking, and has some other improvements we can just enable the use case vs reverting all of this. Thank you for handling this!

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented Nov 4, 2022

@soulxu I think given this wasn't release blocking, and has some other improvements we can just enable the use case vs reverting all of this. Thank you for handling this!

got it, thanks! I will add another pr enable this.

@soulxu soulxu closed this Nov 4, 2022
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.

5 participants