-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Envoy #2639
Update Envoy #2639
Conversation
Signed-off-by: GitHub Action <noreply@github.com>
/retest |
There's a legitimate breakage here that I haven't fully thought through how to handle:
|
It seems to be used in tests only but I have some recollection of this feature flag. I am sure that at some point it was used in config.cc but it does not seem to be used in there anymore. I found that Lyft internal discussion related to that runtime flag https://lyft.slack.com/archives/CLJGGN2JJ/p1647436776083709. Its usage was introduced in #2105 |
I found a PR that removed It looks that it should be safe to remove the usage of that runtime flags from our tests - that should make the PR green. |
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
/retest |
After a little bit more of an investigation. We removed Back to the failures in here. After the removal of
|
Envoy Mobile Logs from a failing test
|
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Copy-pasted @alyssawilk's fix from #2657 in 4fc0f22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ci willing
Description: Remove the use of removed override_request_timeout_by_gateway_timeout upstream Envoy flag from Swift tests. The flag was removed from the main config.cc as part of #2296 file months ago. The original removal introduced a bug as filter's onError callback started being called twice - this PR fixes this issue. Update EM code to correctly handle stream idle. See #2639 for more details. Fixes #2108. Risk Level: Medium, touches stream onDestroy code. Testing: Unit/Integration tests. Docs Changes: N/A Release Notes: N/A Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Closing as #2659 was merged. |
Automated changes by create-pull-request GitHub action