minor opt: minor optimization to the orca parser #36492
Conversation
Signed-off-by: wangbaiping <[email protected]>
|
/wait-any on CI |
There was a problem hiding this comment.
Thanks!
Overall LGTM, modulo minor comment.
Assigning Blake as the original contributor of this code.
/assign @blake-snyder
source/common/orca/orca_parser.cc
Outdated
| #else | ||
| IS_ENVOY_BUG("JSON formatted ORCA header support not implemented for this build"); | ||
| #endif // !ENVOY_ENABLE_FULL_PROTOS || !ENVOY_ENABLE_YAML | ||
| } else { | ||
| return absl::InvalidArgumentError( | ||
| fmt::format("unsupported ORCA header format: {}", split_header.first)); | ||
| return absl::InvalidArgumentError(fmt::format("unsupported ORCA header format")); |
There was a problem hiding this comment.
I'd consider keeping some string as part of the error.
I think it makes sense to keep the minimum between the first 5 characters, and the prefix up to the first occurrence of a " " (the delimiter).
It makes it easier to debug if/when things go wrong with an external upstream server.
|
/wait-any on CI |
|
(yeah I just invited blake to the org but I think it hasn't gone through yet =P) |
Signed-off-by: wangbaiping <[email protected]>
Signed-off-by: wangbaiping <[email protected]>
source/common/protobuf/utility.h
Outdated
There was a problem hiding this comment.
Does it make sense to split changes to the utility files/methods out into another PR as their impact isn't limited to ORCA code?
There was a problem hiding this comment.
Does it make sense to split changes to the utility files/methods out into another PR as their impact isn't limited to ORCA code?
|
block by #36525 |
Signed-off-by: wangbaiping <[email protected]>
Commit Message: minor opt: minor optimization to the orca parser
Additional Description:
By this way, the parser needn't to scan the whole header value if the header value has invalid format. And the we needn't create a copy of the header value for json format now.
Risk Level: low.
Testing: n/a.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.