Skip to content
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

Use single http client for cli #441

Merged
merged 1 commit into from
Sep 15, 2022
Merged

Use single http client for cli #441

merged 1 commit into from
Sep 15, 2022

Conversation

miry
Copy link
Contributor

@miry miry commented Sep 11, 2022

Introduce single Client httpClient for all cli requests.
It allows to add request headers in single place.
For example adds User-Agent HTTP header for all requests as "toxiproxy-cli/<version> <os>/<runtime>".
Parse response on success or errors.
Set content type as json always.

@miry miry self-assigned this Sep 11, 2022
@miry miry added the Toxiproxy label Sep 11, 2022
@miry miry force-pushed the refactoring-client branch 7 times, most recently from 7a2d222 to 062467b Compare September 12, 2022 21:00
@miry miry changed the title Update client Use single http client for cli Sep 12, 2022
@miry miry force-pushed the refactoring-client branch from 003d5dd to 72825a1 Compare September 12, 2022 22:39
@miry miry marked this pull request as ready for review September 12, 2022 23:11
@miry miry added this to the 2.6.0 milestone Sep 12, 2022
client/proxy.go Outdated Show resolved Hide resolved
client/client_test.go Outdated Show resolved Hide resolved
client/client.go Outdated
}
defer resp.Body.Close()

result, err := io.ReadAll(resp.Body)
Copy link

Choose a reason for hiding this comment

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

I wonder if this should be part of send instead, since it's duplicated in 3 other function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review from fresh look and moved also request initiazliation inside and remove checking by specific http status code, so now client.go looks even smaller.

client/client.go Outdated Show resolved Hide resolved
@miry miry force-pushed the refactoring-client branch 2 times, most recently from 74e1ba0 to d178282 Compare September 15, 2022 17:37
Initialize single http client to request api server.
Specify User-Agent and Content-Type.
Parse error response in single place.
@miry miry force-pushed the refactoring-client branch from bf379c2 to 48d5c8a Compare September 15, 2022 18:48
@miry miry merged commit 48d5c8a into master Sep 15, 2022
@miry miry deleted the refactoring-client branch September 15, 2022 18:57
@miry miry mentioned this pull request Sep 15, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants