Skip to content

Conversation

@abeyad
Copy link

@abeyad abeyad commented Feb 2, 2017

This commit changes the exception type thrown when trying to
create a snapshot with a name that already exists in the repository.
Instead of throwing a SnapshotCreateException, which results in a
generic 500 status code, a duplicate snapshot name will throw a
InvalidSnapshotNameException, which will result in a 400 status code
(bad request).

Note that before 5.0, a duplicate snapshot name would throw an
InvalidSnapshotNameException. After #18228, the exception type was
inadvertently changed.

create a snapshot with a name that already exists in the repository.
Instead of throwing a SnapshotCreateException, which results in a
generic 500 status code, a duplicate snapshot name will throw a
InvalidSnapshotNameException, which will result in a 400 status code
(bad request).
@clintongormley
Copy link
Contributor

How about using 409 CONFLICT instead?

@abeyad
Copy link
Author

abeyad commented Feb 2, 2017

@clintongormley I thought about that but then I noticed in our ResourceAlreadyExistsException, it also throws a 400 BAD_REQUEST and not a 409 CONFLICT. Index creation uses the ResourceAlreadyExistsException if the name is already in use, so it throws a 400. I wasn't sure why we didn't throw a 409 there, but I just stuck with the 400 for consistency, and also to not have to introduce a new exception type.

WDYT?

@clintongormley
Copy link
Contributor

OK, let's stick with 400 for now until we have time to review in more depth

@abeyad abeyad requested a review from imotov February 6, 2017 14:55
Copy link
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

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

LGTM

@abeyad abeyad merged commit 42a9f95 into elastic:master Feb 6, 2017
@abeyad
Copy link
Author

abeyad commented Feb 6, 2017

thanks @imotov

@abeyad abeyad removed the v5.2.1 label Feb 6, 2017
@abeyad abeyad deleted the duplicate_snapshot_name_exception branch February 6, 2017 17:43
abeyad pushed a commit that referenced this pull request Feb 6, 2017
create a snapshot with a name that already exists in the repository.
Instead of throwing a SnapshotCreateException, which results in a
generic 500 status code, a duplicate snapshot name will throw a
InvalidSnapshotNameException, which will result in a 400 status code
(bad request).
@abeyad
Copy link
Author

abeyad commented Feb 6, 2017

5.x commit: 6f14949

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.

3 participants