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

http: more human friendly request body #902

Closed
oncilla opened this issue Jan 8, 2021 · 2 comments · Fixed by #903
Closed

http: more human friendly request body #902

oncilla opened this issue Jan 8, 2021 · 2 comments · Fixed by #903

Comments

@oncilla
Copy link
Contributor

oncilla commented Jan 8, 2021

First of all, thank you for the great library.

We are using the https://godoc.org/go.uber.org/zap#AtomicLevel.ServeHTTP endpoint for setting the log levels dynamically.

For operators, this is a bit cumbersome, because they have to specify a valid JSON object.

By default, curl uses application/x-www-form-urlencoded as the content type in a PUT request.
I think it would be nice to accept that as content type in ServeHTTP because it is a lot easier to type.
Following would be the command to set the logging level to debug:

curl -X PUT localhost:3000/log/level -d level=debug

If the content type is not application/x-www-form-urlencoded just keep the previous behavior, and expect a JSON payload.

If this is something that would be accepted, I'm be happy to work on this.
I'm just not sure if this qualifies as a breaking change or not.

@abhinav
Copy link
Collaborator

abhinav commented Jan 8, 2021

Hello, and thank you!
This is completely reasonable.
We would welcome a PR for this.

oncilla added a commit to oncilla/zap that referenced this issue Jan 8, 2021
Support `application/x-www-form-urlencoded` as an additional content
type for `AtomicLevel.ServeHTTP`.

This is the default content type for `curl -X POST`.

With this change, interacting with the HTTP endpoint is a bit more
user friendly:

```
curl -X PUT localhost:8080/log/level -d level=debug
```

Additionally, the unit tests for the HTTP handler are transformed to
a table driven approach.

fixes uber-go#902
@oncilla
Copy link
Contributor Author

oncilla commented Jan 8, 2021

@abhinav alright, PR is out: #903

prashantv pushed a commit that referenced this issue Feb 2, 2021
Support `application/x-www-form-urlencoded` as an additional content
type for `AtomicLevel.ServeHTTP`.

This is the default content type for `curl -X POST`.

With this change, interacting with the HTTP endpoint is a bit more
user friendly:

```
curl -X PUT localhost:8080/log/level -d level=debug
```

Additionally, the unit tests for the HTTP handler are transformed to
a table driven approach.

fixes #902

Co-authored-by: Abhinav Gupta <abg@uber.com>
abhinav added a commit that referenced this issue May 25, 2021
Support `application/x-www-form-urlencoded` as an additional content
type for `AtomicLevel.ServeHTTP`.

This is the default content type for `curl -X POST`.

With this change, interacting with the HTTP endpoint is a bit more
user friendly:

```
curl -X PUT localhost:8080/log/level -d level=debug
```

Additionally, the unit tests for the HTTP handler are transformed to
a table driven approach.

fixes #902

Co-authored-by: Abhinav Gupta <abg@uber.com>
cgxxv pushed a commit to cgxxv/zap that referenced this issue Mar 25, 2022
Support `application/x-www-form-urlencoded` as an additional content
type for `AtomicLevel.ServeHTTP`.

This is the default content type for `curl -X POST`.

With this change, interacting with the HTTP endpoint is a bit more
user friendly:

```
curl -X PUT localhost:8080/log/level -d level=debug
```

Additionally, the unit tests for the HTTP handler are transformed to
a table driven approach.

fixes uber-go#902

Co-authored-by: Abhinav Gupta <abg@uber.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants