-
Notifications
You must be signed in to change notification settings - Fork 232
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
context is not respected for HTTP requests #320
Comments
Hello @zeisss, thanks for opening, do you see the same issue with the v2 branch ? |
I am not sure how to test that, since
But looking at the code at https://github.com/hashicorp/go-getter/blob/v2/get_http.go#L83-L91 I believe the issue is still around. I would expect a call to I can double check my suspicion for v2 if you require it. |
Hey @zeisss, thanks for taking a look, I agree with you conclusion, would you like to open a PR to fix that ? Thanks ! |
@azr for completness sake: Do you want me to make a similar PR for your |
Hey @zeisss, thanks for asking, if you have the time for making one in the |
I created #324 with a fix for the |
I hope its ok to keep this ticket busy a bit longer: Are there any plans to make a release soonish or should I update my project to use |
v1.5.4 was just released ! |
I noticed! Nice and thanks for you help! Definately closed now :) |
It seems the passed
Ctx
togetter.Client
is not passed to thehttp.Request
:https://github.com/hashicorp/go-getter/blob/main/get_http.go#L85-L94
I wrote a test for this below. I receive the context.Canceled error, but it still waits for both requests (HEAD + GET) to finish, thus this test takes 6 seconds instead of <1sec.
Obviously it is more realistic to cancel the context during the request. This could be achieved by moving the
cancel()
call into the http handler.The text was updated successfully, but these errors were encountered: