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.

Sign up for GitHub

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

Jump to bottom

[ZK filter] Remove EnvoyException on data plane 2nd attempt #31485

Merged
merged 4 commits into from
Dec 28, 2023
Merged

[ZK filter] Remove EnvoyException on data plane 2nd attempt #31485

merged 4 commits into from
Dec 28, 2023

Conversation

Winbobob
Copy link
Member

@Winbobob Winbobob commented Dec 21, 2023 •
edited
Loading

Commit Message: [ZK filter] Remove EnvoyException on data plane 2nd attempt
Additional Description:

Risk Level: Low
Testing: Unit test
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

@Winbobob
Reapply envoyproxy#30438
14a8f76 
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
@Winbobob
Bring back exception handling, add stats and add IS_ENVOY_BUG
3fbaf5e 
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
@Winbobob
Add buffer minimal len check
67eb7f9 
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
@Winbobob Winbobob changed the title Remove exceptions on zk proxy filter data plane 2nd attempt [ZK filter] Remove exceptions on zk proxy filter data plane 2nd attempt Dec 21, 2023
@Winbobob Winbobob changed the title [ZK filter] Remove exceptions on zk proxy filter data plane 2nd attempt [ZK filter] Remove EnvoyException on data plane 2nd attempt Dec 21, 2023
@Winbobob
Copy link
Member Author

cc @JuniorHsu, please review if you have a chance, thank you!

JuniorHsu
JuniorHsu suggested changes Dec 22, 2023
View reviewed changes
Copy link
Contributor

@JuniorHsu JuniorHsu left a comment

Choose a reason for hiding this comment

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

only nits, thanks!

source/extensions/filters/network/zookeeper_proxy/decoder.cc Outdated Show resolved Hide resolved
source/extensions/filters/network/zookeeper_proxy/decoder.cc Outdated Show resolved Hide resolved
source/extensions/filters/network/zookeeper_proxy/decoder.cc Outdated Show resolved Hide resolved
source/extensions/filters/network/zookeeper_proxy/utils.cc Outdated Show resolved Hide resolved
source/extensions/filters/network/zookeeper_proxy/utils.h Outdated Show resolved Hide resolved
@Winbobob
Address comments
c7c90dc 
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
JuniorHsu
JuniorHsu approved these changes Dec 22, 2023
View reviewed changes
Copy link
Contributor

@JuniorHsu JuniorHsu left a comment

Choose a reason for hiding this comment

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

thanks!

@mattklein123 mattklein123 merged commit 2991c1d into envoyproxy:main Dec 28, 2023
53 checks passed
@Winbobob Winbobob mentioned this pull request Jan 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@JuniorHsu JuniorHsu JuniorHsu approved these changes

Assignees

@mattklein123 mattklein123

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Winbobob @JuniorHsu @mattklein123