Fix update.Response getting skipped by the proxy#209
Merged
Quinn-With-Two-Ns merged 2 commits intomasterfrom Feb 10, 2025
Merged
Fix update.Response getting skipped by the proxy#209Quinn-With-Two-Ns merged 2 commits intomasterfrom
Quinn-With-Two-Ns merged 2 commits intomasterfrom
Conversation
cretz
approved these changes
Feb 10, 2025
Contributor
cretz
left a comment
There was a problem hiding this comment.
Thanks! Can we open a separate issue to have a test that somehow makes sure every payload is visited even if it's not reachable from workflow service? I can think of a few ways to write it.
Contributor
Author
Contributor
|
Thanks, this LGTM, merge away |
stephanos
approved these changes
Feb 10, 2025
Contributor
Author
|
@cretz Can you please re-review I also noticed we didn't handle any in the failure case |
cretz
approved these changes
Feb 10, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What
Add
update.Responseto proxyWhy?
Without this change payloads under
update.Responseare skipped by the proxy. This can cause update results to not go through the proxy.How did you test it?
New unit test added