fix(entity-query-service): revert bulk update #127
Conversation
This reverts commit 4cffb07.
Bulk update APIs are currently leading to corrupt entity documents. Reverting it back, till we fix the root cause
Codecov Report
@@ Coverage Diff @@
## main #127 +/- ##
============================================
- Coverage 59.71% 59.61% -0.10%
+ Complexity 303 292 -11
============================================
Files 39 39
Lines 3043 2974 -69
Branches 376 368 -8
============================================
- Hits 1817 1773 -44
+ Misses 1053 1029 -24
+ Partials 173 172 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| Map<String, EntityUpdateInfo> entitiesMap = request.getEntitiesMap(); | ||
| try { | ||
| doBulkUpdate(requestContext, entitiesMap); | ||
| responseObserver.onCompleted(); |
There was a problem hiding this comment.
will try to look through this more carefully at some point today, but one thing that jumps out at me off the bat - we never actually onNext a result set chunk here that I can see.
There was a problem hiding this comment.
Discussed a bit offline. There may not be a bug here, but it has a very unintuitive api that can easily lead to corruption - a Value going in looks like a oneof style object but is not, so both the field and type need to be set. If they're not both set, we may be changing data types of persisted data.
There was a problem hiding this comment.
Oh, the API isn't supposed to send any response, as of now. We can just change the API to just send an empty response as of now, instead of result set chunk
There was a problem hiding this comment.
If we're going to break the new api by changing its return type anyway, perhaps it makes sense to fix the value type there?
|
No need to revert the APIs. The API implementation is fine as tested by the integration tests #128. We need to enhance the entity query service to support updates for integer value types |
it should support that - wasn't the issue on the caller side? |
Bulk update APIs are currently leading to corrupt entity documents. Reverting it back, till we fix the root cause