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

Loki: Revert #4845 which changed the format of errors from the API #5772

Merged
merged 2 commits into from
Apr 5, 2022

Conversation

slim-bean
Copy link
Collaborator

What this PR does / why we need it:

PR #4845 changed the return type of error messages from plain text to JSON.

This PR was actually fixing a problem with the current API which returns plain text and sets a Content-Type header of application/json but then returns plain text. The problem is it's significantly changing the behavior of the API which should be considered a breaking change and we should wait to do this at the next major release.

It's entirely possible anyone who's implemented their own code against the Loki API are expecting the error messages in their current format and even though this is a "fix" it would break those users so we want to wait to do this until a future major release.

…nt type from plain text to JSON is a breaking change of the API and we should wait to do this at a major release.

Signed-off-by: Ed Welch <edward.welch@grafana.com>
@slim-bean slim-bean requested review from KMiller-Grafana and a team as code owners April 5, 2022 14:33
@slim-bean slim-bean added the backport release-2.5.x Tag a PR with this label to create a PR which cherry pics it into the release-2.5.x branch label Apr 5, 2022
Signed-off-by: Ed Welch <edward.welch@grafana.com>
@KMiller-Grafana
Copy link
Contributor

Should the change log entry also be reverted?

@slim-bean slim-bean merged commit c158b2c into main Apr 5, 2022
@slim-bean slim-bean deleted the revert-error-api branch April 5, 2022 16:44
grafanabot pushed a commit that referenced this pull request Apr 5, 2022
…5772)

* This reverts the changes from #4845, unfortunately changing the content type from plain text to JSON is a breaking change of the API and we should wait to do this at a major release.

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* fix tests which were updated after initial change.

Signed-off-by: Ed Welch <edward.welch@grafana.com>
(cherry picked from commit c158b2c)
slim-bean added a commit that referenced this pull request Apr 6, 2022
…5772) (#5774)

* This reverts the changes from #4845, unfortunately changing the content type from plain text to JSON is a breaking change of the API and we should wait to do this at a major release.

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* fix tests which were updated after initial change.

Signed-off-by: Ed Welch <edward.welch@grafana.com>
(cherry picked from commit c158b2c)

Co-authored-by: Ed Welch <edward.welch@grafana.com>
splitice pushed a commit to X4BNet/loki that referenced this pull request May 21, 2022
… API (grafana#5772) (grafana#5774)

* This reverts the changes from grafana#4845, unfortunately changing the content type from plain text to JSON is a breaking change of the API and we should wait to do this at a major release.

Signed-off-by: Ed Welch <edward.welch@grafana.com>

* fix tests which were updated after initial change.

Signed-off-by: Ed Welch <edward.welch@grafana.com>
(cherry picked from commit c158b2c)

Co-authored-by: Ed Welch <edward.welch@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-2.5.x Tag a PR with this label to create a PR which cherry pics it into the release-2.5.x branch size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants