Skip to content

use absl::StatusOr#5150

Merged
istio-testing merged 8 commits intoistio:masterfrom
zirain:fix-proxy
Nov 17, 2023
Merged

use absl::StatusOr#5150
istio-testing merged 8 commits intoistio:masterfrom
zirain:fix-proxy

Conversation

@zirain
Copy link
Copy Markdown
Member

@zirain zirain commented Nov 16, 2023 •
edited
Loading

@zirain zirain requested a review from a team November 16, 2023 01:56
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 16, 2023
@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Nov 16, 2023

Can you make sure there are no exceptions as well? StatusOr is meant to replace exceptions.

@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 16, 2023
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 16, 2023
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 16, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 16, 2023

Can you make sure there are no exceptions as well? StatusOr is meant to replace exceptions.

I change this because of envoyproxy/envoy#30765, I don't think need any logic change.

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 16, 2023

/test test-tsan

1 similar comment
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 16, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 16, 2023

/retest

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Nov 17, 2023

/retest

I think it's starting to flake locally too, so maybe we should finally root cause it.

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

can we make these optional before figure out what happen?

@kyessenov
Copy link
Copy Markdown
Contributor

kyessenov commented Nov 17, 2023

Yeah, put SkipTsanAsan on the test TestStackdriverRbacTCPDryRun, it'll let this merge. The test fails on non-Asan so it's just a borked test probably.

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

open istio/test-infra#5093

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-asan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

3 similar comments
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

3 similar comments
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

3 similar comments
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/test test-tsan

@zirain zirain mentioned this pull request Nov 17, 2023
Merged
@zirain
Copy link
Copy Markdown
Member Author

zirain commented Nov 17, 2023

/retest

@istio-testing istio-testing merged commit afbcd5a into istio:master Nov 17, 2023
@zirain zirain deleted the fix-proxy branch November 17, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@kyessenov kyessenov kyessenov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zirain @kyessenov @istio-testing