Skip to content

Conversation

nickbp
Copy link
Contributor

@nickbp nickbp commented Mar 3, 2021

What this PR does:
At the moment the error ends up getting overwritten/obscured by the This should not happen error handling a few lines further down. This ensures that the original error isn't stomped on.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

At the moment the error ends up getting overwritten/obscured by the `This should not happen` error handling.

Signed-off-by: Nick Parker <nick@nickbp.com>
Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Out of curiosity: how did you spot it? This code is part of a feature still under development which doesn't work out of the box yet.

@pstibrany pstibrany merged commit 9c46081 into cortexproject:master Mar 3, 2021
@spahl
Copy link

spahl commented Mar 3, 2021

I'll answer since Nick is sleeping ;-)

We were working on integrating the alertmanager and ruler in Opstrace (opstrace/opstrace#430) the thread is rather long and show us stumbling around haha:-)

We are working on cortex master and would rather use and test what comes out there since we are not in a rush.

@pracucci
Copy link
Contributor

pracucci commented Mar 3, 2021

Fantastic!

We are working on cortex master and would rather use and test what comes out there since we are not in a rush.

Just keep in mind the alertmanager sharding is not ready yet. There are still missing pieces we're working on. Since the work is huge, we're splitting it up in many PRs which get progressively merged to master.

@spahl
Copy link

spahl commented Mar 3, 2021

yeah we disabled it:-) Thanks!

roystchiang pushed a commit to roystchiang/cortex that referenced this pull request Apr 6, 2022
At the moment the error ends up getting overwritten/obscured by the `This should not happen` error handling.

Signed-off-by: Nick Parker <nick@nickbp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants