Batch 12 of #2285, converting shredding-related ErrorCodeV1 entries
#2322
Conversation
📈 Unit Test Coverage Delta vs Main Branch
|
Unit Test Coverage Report
|
📉 Integration Test Coverage Delta vs Main Branch (hcd-it)
|
Integration Test Coverage Report (hcd-it)
|
📉 Integration Test Coverage Delta vs Main Branch (dse69-it)
|
Integration Test Coverage Report (dse69-it)
|
| return OPTIONS; | ||
| } | ||
| throw ErrorCodeV1.INVALID_PARAMETER_VALIDATION_TYPE.toApiException(type); | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
One-off failure not to be directly exposed: caller will wrap/convert validation problems in more centralized way.
There was a problem hiding this comment.
I am guessing we only need this stuff because of the EGW and the trouble we have with configs ?
There was a problem hiding this comment.
I am not sure why this was added: I don't think non-contextual, Enum lookups should throw Data API exceptions; they can't add much context information. I'll see what caller does.
There was a problem hiding this comment.
Uh. It's called from just one place but that's a Record constructor in ParameterConfigImpl:
public ParameterConfigImpl(
EmbeddingGateway.GetSupportedProvidersResponse.ProviderConfig.ParameterConfig
grpcModelParameter) {
this(
grpcModelParameter.getName(),
ParameterType.valueOf(grpcModelParameter.getType().name()),
grpcModelParameter.getRequired(),
Optional.of(grpcModelParameter.getDefaultValue()),
grpcModelParameter.getValidationMap().entrySet().stream()
.collect(
Collectors.toMap(
e -> ValidationType.fromString(e.getKey()),
e -> new ArrayList<>(e.getValue().getValuesList()))),
will leave as-is, for now.
tatu-at-datastax
changed the title
ErrorCodeV1 entriesErrorCodeV1 entries
amorton
left a comment
amorton
left a comment
There was a problem hiding this comment.
some nit picks, and we should pull out some things for me to polish after the merge. see comments.
Biggest questions are in DocumentId where it is decoding from DB
| INVALID_COLUMN_VALUES, | ||
| MISSING_PRIMARY_KEY_COLUMNS, | ||
|
|
||
| SHRED_BAD_BINARY_VECTOR_VALUE, |
There was a problem hiding this comment.
we dont have any BAD error codes in V2 errors - we can let them through and then I can fix in a follow up ? as in they take some thinking do we want to do that now ?
There was a problem hiding this comment.
Agreed, postpone for now to give us more time to think it through.
| return OPTIONS; | ||
| } | ||
| throw ErrorCodeV1.INVALID_PARAMETER_VALIDATION_TYPE.toApiException(type); | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
I am guessing we only need this stuff because of the EGW and the trouble we have with configs ?
src/main/java/io/stargate/sgv2/jsonapi/service/shredding/collections/DocumentId.java
Outdated
Show resolved
Hide resolved
| Document field name not valid: ${errorMessage}. | ||
| - scope: DOCUMENT | ||
| code: SHRED_BAD_VECTOR_SIZE |
There was a problem hiding this comment.
I know this is a copy of the old error - tagging that it needs work... there will also be places where the array is non zero but not the expected size.
There was a problem hiding this comment.
Yes. Probably follow-up concern?
What this PR does:
Converts yet more
ErrorCodeV1entries to new Error entitiesWhich issue(s) this PR fixes:
Part of #2285
Checklist