runtime: load rtds bool correctly as true/false instead of 1/0 #36044
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @gordon-wang-lyft, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
gordon-wang-lyft
changed the title
gordon-wang-lyft
force-pushed
the
main
branch
2 times, most recently
from
c233209 to
9f47dbb
Compare
alyssawilk
reviewed
Sep 10, 2024
| )EOF"); | ||
| EXPECT_CALL(rtds_init_callback_, Call()); | ||
| doOnConfigUpdateVerifyNoThrow(runtime); | ||
|
|
||
| EXPECT_EQ("bar", loader_->snapshot().get("foo").value().get()); | ||
| EXPECT_EQ("yar", loader_->snapshot().get("bar").value().get()); | ||
| EXPECT_EQ("meh", loader_->snapshot().get("baz").value().get()); | ||
| EXPECT_EQ("true", loader_->snapshot().get("toggle").value().get()); |
There was a problem hiding this comment.
just to confirm, this check fails prior to your change?
There was a problem hiding this comment.
Yes. This is a newly added check.
Without the source code change, it will fail, since snapshot().get("toggle").value().get() will return "1" instead of "true".
There was a problem hiding this comment.
Also updated a comment so it is more clear why absl::StrCat is not used for boolean case
gordon-wang-lyft
force-pushed
the
main
branch
from
9f47dbb to
44bfa84
Compare
gordon-wang-lyft
force-pushed
the
main
branch
from
44bfa84 to
ba5eb78
Compare
alyssawilk
reviewed
Sep 10, 2024
|
/retest |
Will add and update soon. |
gordon-wang-lyft
force-pushed
the
main
branch
from
ba5eb78 to
b6e934f
Compare
gordon-wang-lyft
force-pushed
the
main
branch
3 times, most recently
from
9b6d4ac to
d1efd5b
Compare
Updated with runtime guard and specific test cases. Let me know how it looks like. Thanks @alyssawilk ! |
gordon-wang-lyft
force-pushed
the
main
branch
from
d1efd5b to
0b79a5b
Compare
…d of "0"/"1" Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
Signed-off-by: Gordon Wang <[email protected]>
gordon-wang-lyft
force-pushed
the
main
branch
from
0b79a5b to
ee84b3e
Compare
alyssawilk
approved these changes
Sep 11, 2024
unicell
added a commit
to unicell/envoy
that referenced
this pull request
Sep 11, 2024
* upstream/main: (21 commits) Add a CPU utilization resource monitor for overload manager (envoyproxy#34713) jwks: Add UA string to headers (envoyproxy#35977) exceptions: cleaning up macros (envoyproxy#35694) coverage: ratcheting (envoyproxy#36058) runtime: load rtds bool correctly as true/false instead of 1/0 (envoyproxy#36044) Typo in documentation of http original_src filter (envoyproxy#36060) docs: updating meeting info (envoyproxy#36052) quic: removes more references to spdy::Http2HeaderBlock. (envoyproxy#36057) json: add null support to the streamer (envoyproxy#36051) json: make the streamer a template class (envoyproxy#36001) docs: Add `apt.envoyproxy.io` install information (envoyproxy#36050) ext_proc: elide redundant copy in ext_proc filter factory callback (envoyproxy#36015) build(deps): bump yarl from 1.11.0 to 1.11.1 in /tools/base (envoyproxy#36049) build(deps): bump multidict from 6.0.5 to 6.1.0 in /tools/base (envoyproxy#36048) quic: enable certificate compression/decompression (envoyproxy#35999) Geoip fix asan failure (envoyproxy#36043) mobile: Fix missing logging output in Swift integration tests (envoyproxy#36040) http: minor code clean up to the http filter manager (envoyproxy#36027) ci/example: Dont build/test the filter example in Envoy CI (envoyproxy#36038) ci/codeql: Fix build setup (envoyproxy#36021) ... Signed-off-by: Qiu Yu <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message: Load RTDS boolean config correctly as true/false instead of 1/0
Additional Description: Currently, RTDS boolean values are being loaded as
1or0, which is inconsistent with the expectedtrueorfalsevalues. This discrepancy needs to be corrected so that boolean values are consistently loaded and represented astrueorfalse. For further details, please refer to the issue comment where this inconsistency is discussed.Risk Level: Low (to be consistent with initially loaded true/false value)
Testing: new Unit Test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
[Optional Fixes #Issue] #35762