Skip to content

Conversation

gouthamve
Copy link
Contributor

We current reply with a 200 even if bad config is applied. This
fixes that.

Signed-off-by: Goutham Veeramachaneni [email protected]

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.

Could you add a CHANGELOG entry, please?

@gouthamve gouthamve force-pushed the validate-am-config branch 2 times, most recently from b634a40 to 3251441 Compare August 18, 2020 13:50
Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM.

Copy link
Contributor

@jtlisi jtlisi left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Good job @gouthamve! I left few nits I would be glad if could take a look, but consider it already approved from me.

Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

My comments are nits and entirely optional. LGTM from here, thanks for taking care of my feedback!

We current reply with a 200/201 even if bad config is applied. This
fixes that.

Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
@gouthamve gouthamve merged commit 04ec413 into cortexproject:master Aug 21, 2020
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