fix: Read file-like KVS values before upload and reject unencodable ones - #965
fix: Read file-like KVS values before upload and reject unencodable ones#965vdusek wants to merge 10 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #965 +/- ##
==========================================
- Coverage 94.66% 94.65% -0.01%
==========================================
Files 58 58
Lines 5248 5259 +11
==========================================
+ Hits 4968 4978 +10
- Misses 280 281 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| """ | ||
| value, content_type = encode_key_value_store_record_value(value, content_type=content_type) | ||
| # Encoding may read a file or serialize a large payload (blocking), so run it off the event loop. | ||
| value, content_type = await asyncio.to_thread( |
There was a problem hiding this comment.
asyncio.to_thread is useful only to the branch that can do IO. It just adds overhead in other cases, or? So it improves the less frequent case at the cost of the main use case
There was a problem hiding this comment.
Well, AFAIK the criterion is more whether the callee releases the GIL. If it does, the worker thread can work on the offloaded task, while the main thread, which runs the event loop, can run other asyncio tasks.
And here is the problem: json.dumps, which is called in encode_key_value_store_record_value, never releases the GIL. So yeah, let's remove it.
Thanks.
Passing a file-like value (
open(...),io.BytesIO(...)) toset_record— or toactor.start/actor.call/actor.validate_input/run.metamorphviarun_input— crashed with a rawTypeError: 'BytesIO' object is not an instance of 'Sequence'.encode_key_value_store_record_valueset a content type forio.IOBasebut passed the object through unread, and impit'scontent=only accepts bytes-like bodies.The encoder now reads file-like values into memory. Detection is duck-typed on a callable
read, so non-io.IOBasewrappers work too. Binary maps toapplication/octet-stream, text totext/plain.read()is called with no arguments, so the value is consumed from its current position and the object is neither rewound nor closed.Values it cannot turn into a body now raise
TypeErrorinstead of reaching the transport:aiofiles, StarletteUploadFile) —read()returns a coroutineread()returning neither bytes norstr, e.g.Nonefrom a non-blocking raw streamset_record('k', {'a': 1}, content_type='image/png')Note: impit exposes no streaming
content=API, so uploads are fully buffered rather than streamed.Tests:
set_recordoverBytesIO/StringIO/ a duck-typed reader, sync and async, over both compression algorithms, plus encoder coverage of the three rejection paths.✍️ Drafted by Claude Code