Query: correctly marshal errors to JSON and ignore if nil #2848
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kakkoyun
reviewed
Jul 7, 2020
There was a problem hiding this comment.
lgtm. Let's see what @GiedriusS thinks.
Just a small nit on changelog entry.
Have you tested this on a running system?
|
Ah no problem. I'll take the entry out the changelog.
I've tested on a locally compiled binary talking to a real 0.13 Sidecar.
…On Tue, 7 Jul 2020, 09:27 Kemal Akkoyun, ***@***.***> wrote:
***@***.**** commented on this pull request.
lgtm. Let's see what @GiedriusS <https://github.com/GiedriusS> thinks.
Just a small nit on changelog entry.
Have you tested this on a running system?
------------------------------
In CHANGELOG.md
<#2848 (comment)>:
> @@ -16,6 +16,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel
* [#2665](#2665) Swift: fix issue with missing Content-Type HTTP headers.
- [#2800](#2800) Query: Fix handling of `--web.external-prefix` and `--web.route-prefix`
- [#2834](#2834) Query: Fix rendered JSON state value for rules and alerts should be in lowercase
+- [#2848](#2848) Query: Fix remove erroneous error when viewing stores.
We don't need this entry. Since this is a fix for a change that hasn't
merged yet.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2848 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXFAGPBAFJSR3EW77ZYPZ3R2LL6HANCNFSM4OSAHHOA>
.
|
bwplotka
approved these changes
Jul 7, 2020
Signed-off-by: Dan Potepa <[email protected]>
GiedriusS
reviewed
Jul 7, 2020
Signed-off-by: Dan Potepa <[email protected]>
GiedriusS
reviewed
Jul 7, 2020
|
@GiedriusS I have requested him to remove it since this hasn't released yet. That reminded me maybe we should change the base branch to |
|
correct |
|
@cuotos Then could you branch out from |
|
But are there any practical differences between merging this into |
|
Right, let's merge. |
kakkoyun
pushed a commit
that referenced
this pull request
Jul 7, 2020
* correctly marshal errors to JSON and ignore if nil Signed-off-by: Dan Potepa <[email protected]> * LastError should be cleared if the newer update was ok Signed-off-by: Dan Potepa <[email protected]> Signed-off-by: Kemal Akkoyun <[email protected]>
paulfantom
added a commit
to paulfantom/thanos
that referenced
this pull request
Jul 9, 2020
* upstream/release-0.14: (46 commits) Cut release v0.14.0-rc.1 (thanos-io#2853) Query: correctly marshal errors to JSON and ignore if nil (thanos-io#2848) ci: Manually download promu in crossbuild stage (thanos-io#2828) Cut release v0.14.0-rc.0 (thanos-io#2826) Soft cut changelog on master to indicate v0.14.0 being in progress (thanos-io#2824) Update ThanosReceiveNoUpload to select sum == 0 (thanos-io#2819) receive: Added more observability, fixed leaktest, to actually check leaks ): (thanos-io#2817) Query: always return a string in the `lastError` field (thanos-io#2809) Added missing CHANGELOG entry for PR 2613 (thanos-io#2820) receive: Fixed small options race; Removed unused StartTime feature. (thanos-io#2816) go.mod: Bump Prometheus to current latest (thanos-io#2814) Implement CLI Flags page in React UI (thanos-io#2796) Improve ThanosReceiveNoUpload to only alert on current instances store: Preallocate output buffer when encoding postings. (thanos-io#2812) compact: introduce flag --block-viewer.global.sync-block-interval (thanos-io#2752) docs: compact: add blurb about how retention policy works (thanos-io#2808) Reduced memory allocations in readIndexRange() (thanos-io#2807) ui: Add Stores page to React UI (thanos-io#2754) Added Kemal to Maintainer Role; Kemal is volounteering to be next release shephard (thanos-io#2804) proposal: Add scalable rule storage proposal (thanos-io#2661) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Fixes: #2846
Passing nil error to updateStoreStatus causes a panic. This is displayed to the user in the Stores ui when the Store is healthy
Added higher level unit test to catch this, and hopefully clarified existing test.