-
Notifications
You must be signed in to change notification settings - Fork 16
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
index creation failure msg, usage with collection not fully indexed #879
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor suggestion
src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
...in/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/CreateCollectionOperation.java
Outdated
Show resolved
Hide resolved
src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like improvements here, but I think we can probably improve on wording & inclusion of information (collection name should be included in all, I think). And for "missing index" case indicate it as such (instead of suggesting operation that failed was create-collection -- if I understood logic correctly).
src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableToErrorMapper.java
Show resolved
Hide resolved
...in/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/CreateCollectionOperation.java
Outdated
Show resolved
Hide resolved
src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableToErrorMapper.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There are chances of the collection creation is successful, but not fully indexed.
Even though we now get less failures because index creation changes into serial execution.
This PR will throw a JsonApiException, when there is table creation failure or index creation failure.
Also, if user is using a not fully indexed collection, also prompt with proper error msgs.
Which issue(s) this PR fixes:
Fixes #812
Checklist