-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow streaming of file content #588
Allow streaming of file content #588
Comments
Thanks for reporting this. If we look at the equivalent from the dotnet SDK it returns a stream (which would be equivalent to ReadWriteSeeker + Closer) http.response body returns a ReadCloser, so piping things to a buffer should not be difficult. The only challenge with this improvement is that it'd represent a breaking change to the API surface, so we'll probably need to wait the next major version (no plans at this point). I've created an issue on the generator to track that major change microsoft/kiota#3402 |
to clarify my prior point, even if we added overloads + deprecated comments on existing API surface for executor methods that either take a []byte as a request body or return []byte as a response body, it'd still only be part of the required change. |
In the short term, maybe we can update the This will be non-breaking from our perspective and the rest of the breaking changes can be done later on which will end up calling the function with the updated type name. |
So you'd effectively
This way all streaming scenarios with large payloads would not have memory pressure anymore, we'd maintain compatibility, the only downside is the size of the SDK increasing slightly. correct? I like this approach, it's not perfect, but it unblocks customers. @rkodev, can you double check the priority of this relative to other things with @maisarissi please? |
I was thinking more towards a middle ground without concerns about increasing the size by generation of extra methods by initially providing the means of using
Making the extra step of generation of the extra methods and deprecation of current one, could possibly evaluated separately. |
if we don't generate the overloads, the gains will be hard to discover for customers (they'll effectively have to re-implement the executor method) |
This is how I worked around the problem. Hope it helps before the official streaming support.
|
ItemItemsItemContentRequestBuilder::Get currently reads an entire file into memory and returns it as a
[]byte
slice. This is a problem when dealing with lots of large files. It would be helpful if there were a way to get anio.Reader
so that the response body can be processed as a stream.The text was updated successfully, but these errors were encountered: