Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #787 +/- ##
==========================================
- Coverage 94.51% 94.46% -0.05%
==========================================
Files 395 396 +1
Lines 25479 25650 +171
==========================================
+ Hits 24081 24231 +150
- Misses 1398 1419 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| operation, ``False`` otherwise. | ||
| """ | ||
| check_not_none(key, "key can't be None") | ||
| check_not_none(key, "value can't be None") |
There was a problem hiding this comment.
I assume this should check the value since key is already checked. The same issue also exists in proxy/multi_map.py.
| except SchemaNotReplicatedError as e: | ||
| return await self._send_schema_and_retry(e, self.remove, key, value) | ||
|
|
||
| request = multi_map_remove_entry_codec.encode_request(self.name, key_data, value_data, 0) |
There was a problem hiding this comment.
Thread ids are set to 0 while proxy/multi_map.py uses thead_id(). Is using 0 here valid? If yes, could you please make them constant instead of a magic number?
There was a problem hiding this comment.
Since asyncio client is not supposed to be accessed from different threads, the thread ID isn't useful. Go Client uses 0 as the default thread ID, and Node.js uses 1. I opted for 0.
Introduced the default_thread_id in 3868635
I'll send another PR fro Map.
|
|
||
| Warning: | ||
| The list is NOT backed by the multimap, so changes to the map are | ||
| list reflected in the collection, and vice-versa. |
There was a problem hiding this comment.
Is this sentence correct? I know it's from proxy/multi_map.py but same confusion is there as well.
There was a problem hiding this comment.
Fixed the typo at: 3868635
It just warns the user that, modifying the returned value will not update the multi-map on the server-side.
3 participants
Straightforward port of asyncore MultiMap to asyncio.
Does not include lock-related functions, since they are missing from the asyncio Map as well.