Skip to content

fix: client prompt return on context cancellation - #1729

Merged
kakkoyun merged 4 commits into
prometheus:mainfrom
umegbewe:fix/client-context-canceled
May 16, 2025
Merged

fix: client prompt return on context cancellation#1729
kakkoyun merged 4 commits into
prometheus:mainfrom
umegbewe:fix/client-context-canceled

Conversation

@umegbewe

@umegbewe umegbewe commented Feb 3, 2025

Copy link
Copy Markdown
Contributor

This fixes the issue where the client blocks for the entire duration of the response stream, even when the provided context had already been canceled or times out

Relevant issue #1698

@bwplotka bwplotka left a comment

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.

Thanks! Some questions, not sure if this will work. My understanding was that we have to manually chunk reads from the resp.Body.

Comment thread api/client.go
@kakkoyun
kakkoyun requested a review from bwplotka April 10, 2025 08:27
@kakkoyun

Copy link
Copy Markdown
Member

@umegbewe The PR need to rebase.
I have also pinged @bwplotka to have another look at it.

Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>
Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>
@umegbewe
umegbewe force-pushed the fix/client-context-canceled branch from 9e16398 to 9a59353 Compare April 29, 2025 12:59
Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>
Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>
@umegbewe

Copy link
Copy Markdown
Contributor Author

@bwplotka @kakkoyun updated, PR is good to go

@kakkoyun kakkoyun left a comment

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.

LGTM

@kakkoyun
kakkoyun merged commit 7ed72b7 into prometheus:main May 16, 2025
juliusmh pushed a commit to juliusmh/client_golang that referenced this pull request Jun 30, 2025
* fix: client prompt return on context cancellation

Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>

* test: add context cancellation unit test

Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>

* fix/rid unused package

Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>

* fix/lint and formatting

Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>

---------

Signed-off-by: Umegbewe Nwebedu <nwebedujunior55@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants