Begin process of removing singleton use by StringMatcher #32829
Conversation
Signed-off-by: Greg Greenway <[email protected]>
doesn't use singletons. Add Regex to the factory context interface. Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
ggreenway
requested review from
RyanTheOptimist,
alyssawilk,
abeyad and
fredyw
as code owners
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
|
/retest |
Signed-off-by: Greg Greenway <[email protected]>
Signed-off-by: Greg Greenway <[email protected]>
Yep, some instances where I converted a nullptr to a reference. The reference was never used, but still gets flagged by asan. I think I've worked around it now, but waiting on CI. There was also a real failure on mobile that I think I've fixed. |
Signed-off-by: Greg Greenway <[email protected]>
`regexEngine()` isn't yet called in the config validation path that tests hit Signed-off-by: Greg Greenway <[email protected]>
|
CC @envoyproxy/coverage-shephards: FYI only for changes made to |
| @@ -63,7 +63,7 @@ declare -a KNOWN_LOW_COVERAGE=( | |||
| "source/extensions/listener_managers/validation_listener_manager:70.5" | |||
| "source/extensions/watchdog/profile_action:83.3" | |||
| "source/server:91.0" # flaky: be careful adjusting. See https://github.com/envoyproxy/envoy/issues/15239 | |||
| "source/server/config_validation:89.2" | |||
There was a problem hiding this comment.
Any idea why this changed/how hard it would be to improve?
There was a problem hiding this comment.
I added regexEngine() to the Server::Instance. This isn't used (yet) in the config_validation path, because most of the uses of StringMatcherImpl still don't use it. Future PRs will convert all of those, and this line will get hit in coverage at that time.
|
/retest |
…oyproxy#32829)" This reverts commit b7818b0. Signed-off-by: Ryan Northey <[email protected]>
#32902) Revert "Begin process of removing singleton use by StringMatcher (#32829)" This reverts commit b7818b0. Signed-off-by: Ryan Northey <[email protected]>
Commit Message: Begin process of removing singleton use by StringMatcher
This PR temporarily adds two versions of StringMatcherImpl: one that uses the existing singletons, and one that takes a factory context.
Subsequent PRs will gradually convert all uses of StringMatcherImpl to use the factory context. Then the version using singletons will be deleted, along with the code that sets the singletons.
Additional Description: This work is being broken into multiple PRs because otherwise it would be very large and difficult to review.
Part of #32792
Risk Level: low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]