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

api/v2: Delete silence respond with 404 when silence is not found #2815

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hateeyan
Copy link

Hi,
When a client attempts to delete a non-existent silence, respond with 404 instead of 500.

Signed-off-by: hateeyan hateeyan@gmail.com

Signed-off-by: hateeyan <hateeyan@gmail.com>
Copy link
Member

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

lgtm

@rohang98
Copy link
Contributor

rohang98 commented Apr 18, 2023

@simonpasquier @hateeyan can this be merged?

@emanlodovice
Copy link
Contributor

I tried rebasing this to main and it seems like there are test failures, @hateeyan can you rebase this add fix the failures? Thank you.

@hateeyan
Copy link
Author

hateeyan commented May 4, 2023

@rohang98 @emanlodovice
Another related PR(#2817) has been merged which repond with 200 when silence is not found.
I suppose that this PR is no longer needed.

@jeromeinsf
Copy link

is expired silence the same thing as not found silence?

@emanlodovice
Copy link
Contributor

@hateeyan I think the PR is still needed because expired silence is not the same as not found silence. I created another PR that rebases this PR yesterday. #3352

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants