Skip to content

config: track message ancestors for unknown fields#20055

Merged
mattklein123 merged 15 commits intoenvoyproxy:mainfrom
timonwong:track-msg-path
Mar 10, 2022
Merged

config: track message ancestors for unknown fields#20055
mattklein123 merged 15 commits intoenvoyproxy:mainfrom
timonwong:track-msg-path

Conversation

@timonwong
Copy link
Copy Markdown
Contributor

@timonwong timonwong commented Feb 19, 2022

Signed-off-by: Tianpeng Wang tpwang@alauda.io

Commit Message: config: track message ancestors for unknown fields
Additional Description:
Risk Level: Low
Testing: unit test
Docs Changes: N/A
Release Notes: Required
Platform Specific Features: N/A

@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #20055 was opened by timonwong.

see: more, trace.

@timonwong timonwong force-pushed the track-msg-path branch 2 times, most recently from 6a0cce8 to 2a04beb Compare February 19, 2022 04:21
@timonwong timonwong changed the title protobuf: print message path for unknown fields WIP protobuf: print message path for unknown fields Feb 19, 2022
@timonwong timonwong changed the title WIP protobuf: print message path for unknown fields WIP protobuf: track message path for unknown fields Feb 19, 2022
@timonwong timonwong force-pushed the track-msg-path branch 24 times, most recently from edd5dca to 35c8170 Compare February 20, 2022 04:35
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice, this is a great improvement, thanks. Just some small comments. Thank you.

/wait


if (inner_message != nullptr) {
traverseMessageWorker(visitor, *inner_message, true, recurse_into_any);
ScopedMessageParents scoped_parents(parents, message);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not 100% positive, but I think you want *inner_message here? Since otherwise I think this will show up as an Any vs. the resolved type? Or you could push both the Any and the resolved type. I'm not sure. Either way, can you add a test that uses recurse_into_any and an example that would do such recursion and verify this looks correct?

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@timonwong
Copy link
Copy Markdown
Contributor Author

/wait

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@timonwong
Copy link
Copy Markdown
Contributor Author

/wait

1 similar comment
@timonwong
Copy link
Copy Markdown
Contributor Author

/wait

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@timonwong
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #20055 (comment) was created by @timonwong.

see: more, trace.

@timonwong
Copy link
Copy Markdown
Contributor Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #20055 (comment) was created by @timonwong.

see: more, trace.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

@mattklein123 mattklein123 merged commit a9c3a96 into envoyproxy:main Mar 10, 2022
@timonwong timonwong deleted the track-msg-path branch March 11, 2022 00:57
JuniorHsu pushed a commit to JuniorHsu/envoy that referenced this pull request Mar 17, 2022
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Signed-off-by: kuochunghsu <kuochunghsu@pinterest.com>
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants