[FIXED] Pull requests: don't send 408 when request expires #2482
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
When expiring requests, the server would send 408 if interest was still present, which can happen for pull subscribe implementations that maintain interest for the duration of the pull subscription. Let's keep the 408 for when a request is "force expired", that is, a request was removed from the queue because it queue was full but interest is still found. Signed-off-by: Ivan Kozlovic <[email protected]>
derekcollison
approved these changes
Sep 1, 2021
Signed-off-by: Ivan Kozlovic <[email protected]>
|
Very glad to see this removed LGTM |
|
@ripienaar It is still sent (as of now) if the request is "force expired", that is, the server could not remove (due to expiration) a single request and therefore evicts the first from the queue. In that case, if there is interest, server will still send 408, which I would argue is wrong "status/description", since in this case it is not a timeout but an eviction or some other error, but that would definitively break clients if we introduce a new code there. |
|
Yes, my moan about them was the ones that came at some random future time and not at the point of expiration timer expiring (the lazy ones). So happy to see those specific ones go away. |
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.
When expiring requests, the server would send 408 if interest was
still present, which can happen for pull subscribe implementations
that maintain interest for the duration of the pull subscription.
Let's keep the 408 for when a request is "force expired", that
is, a request was removed from the queue because it queue was
full but interest is still found.
Signed-off-by: Ivan Kozlovic [email protected]