feat: add upsert all support #72
Conversation
Codecov Report
@@ Coverage Diff @@
## main #72 +/- ##
============================================
- Coverage 85.37% 84.20% -1.18%
- Complexity 276 286 +10
============================================
Files 36 36
Lines 1361 1412 +51
Branches 46 45 -1
============================================
+ Hits 1162 1189 +27
- Misses 170 194 +24
Partials 29 29
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
aaron-steinfeld
requested review from
GurtejSohi,
skjindal93 and
surajpuvvada
| * @param userId | ||
| * @return the upserted configs | ||
| */ | ||
| List<UpsertedConfig> writeAllConfigs( |
There was a problem hiding this comment.
what is the error handling behavior ?
There was a problem hiding this comment.
Currently, it's identical to calling N write operations sequentially. If a later one fails, it aborts at that point leaving the original (this is the same behavior in the current abstraction one layer up in ObjectStore). Ideally, with a doc store API change we can make this transactional with all or nothing semantics (this came up with Entity Service in conversations with @saxenakshitiz too, but I believe as of today doc store itself doesn't support this - another reason I paused this change where I did).
| public List<UpsertedConfig> writeAllConfigs( | ||
| Map<ConfigResourceContext, Value> resourceContextValueMap, String userId) throws IOException { | ||
| // TODO keep pushing this down into doc store | ||
| List<UpsertedConfig> list = new ArrayList<>(resourceContextValueMap.size()); |
There was a problem hiding this comment.
so would docstore be able to leverage bulk upsert APIs ?
There was a problem hiding this comment.
That's the idea - just split the PR. The current PR handles all the API changes, which is a fairly significant refactor, but doesn't really change the implementation in any meaningful way. A future PR needs only swap out this implementation to use the doc store's bulk upsert.
Description
Added support for a new upsertAll api. This can be used for upserting multiple configs in fewer calls. Eventually, we'll optimize its internals to also reduce the roundtrips to the persistence store, but this change encapsulates that tech debt.
Testing
Added and updated existing UTs
Checklist: