Skip to content

Copy HTTP client#261

Merged
nhooyr merged 3 commits into
coder:masterfrom
swithek:copy-http-client
Oct 2, 2020
Merged

Copy HTTP client#261
nhooyr merged 3 commits into
coder:masterfrom
swithek:copy-http-client

Conversation

@swithek

@swithek swithek commented Sep 22, 2020

Copy link
Copy Markdown
Contributor

Closes #260

@swithek swithek requested a review from nhooyr as a code owner September 22, 2020 10:57
@swithek swithek changed the title Copy http client Copy HTTP client Sep 22, 2020

@nhooyr nhooyr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing @swithek!

Thoughts on instead of removing the timeout completely, copying the client and removing it in Dial and then using whatever timeout is on the client on the Dial context?

That way things would just work as expected.

@swithek

swithek commented Sep 24, 2020

Copy link
Copy Markdown
Contributor Author

@nhooyr that's a good point, I will submit an update soon.

@swithek swithek requested a review from nhooyr September 24, 2020 07:38

@nhooyr nhooyr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great otherwise!

Comment thread dial.go Outdated
@swithek swithek requested a review from nhooyr September 27, 2020 08:39
@swithek

swithek commented Sep 30, 2020

Copy link
Copy Markdown
Contributor Author

@nhooyr is there anything else that needs improving?

@nhooyr

nhooyr commented Oct 2, 2020

Copy link
Copy Markdown
Contributor

Nope, looks great.

Thanks for contributing @swithek <3

@nhooyr nhooyr merged commit b00726c into coder:master Oct 2, 2020
@dpastoor dpastoor mentioned this pull request Oct 9, 2020
nhooyr added a commit that referenced this pull request Jan 9, 2021
Allow *http.Client with Timeout set
nhooyr added a commit that referenced this pull request Jan 9, 2021
nhooyr added a commit that referenced this pull request Jan 9, 2021
There were a few PRs merged into the master branch that were then not
merged into the dev branch. This branch merges those changes in cleanly.

- #261
- #266
- #273
@nhooyr nhooyr mentioned this pull request Jan 9, 2021
nhooyr added a commit that referenced this pull request Jan 9, 2021
There were a few PRs merged into the master branch that were then not
merged into the dev branch. This branch merges those changes in cleanly.

- #261
- #266
- #273
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error should not be returned on non-zero http client timeout

2 participants