Allow streaming of file content

[eric-millin]

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 an io.Reader so that the response body can be processed as a stream.

[baywet]

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

[baywet]

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 case of the request body, we'd also need to update the content field of the request information currently []byte) and the types in ParseNode constructor, SerializationWriter GetSerializedContent.
The later is really the block because simply changing the return type would be a breaking change, and adding an overload method to an interface would be as well.
This definitively needs to wait for the next major version, sorry about the miss here and the resulting memory pressure.

[andrueastman]

In the short term, maybe we can update the sendPrimitive method to allow another type name (io.ReadWriter) so that in a situation where memory pressure is real concern, the user could call the ToGetRequestInformation method and then pass it to the sendPrimitive method with the added type name as a way out.

https://github.com/microsoft/kiota-http-go/blob/51c8f263d7ad3dabe446c1e3e7f1c776ce7edaaf/nethttp_request_adapter.go#L568

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.

[baywet]

So you'd effectively

1. move the conversion from []byte to readwritercloser from the request adapter to request information
2. add the additional field in request information, deprecate the old one.
3. generate overrides for anything that accepts a []byte on the fluent API surface
4. have the set content implementations in request information write directly to the new property (and rule the temporary []byte for structured payloads a thing to cleanup in the next major version + low impact on memory)

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?

[andrueastman]

So you'd effectively

1. move the conversion from []byte to readwritercloser from the request adapter to request information
2. add the additional field in request information, deprecate the old one.
3. generate overrides for anything that accepts a []byte on the fluent API surface
4. have the set content implementations in request information write directly to the new property (and rule the temporary []byte for structured payloads a thing to cleanup in the next major version + low impact on memory)

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 readwritercloser in the abstractions/requestadapter and documenting the limitation.
This at least provides a way out for the user who can write custom code that

• calls the request generator e.g ToGetRequestInformation to create a requestInfo object.
• pass it to the sendPrimitive method with the readwritercloser as the primitive type

Making the extra step of generation of the extra methods and deprecation of current one, could possibly evaluated separately.

[baywet]

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)

[wanghq]

This is how I worked around the problem. Hope it helps before the official streaming support.

func (m *microsoftService) GetOnlineMeetingRecordingContentStream(
	ctx context.Context, orgGUID, userGUID, onlineMeetingID, recordingID string) (io.ReadCloser, error) {

	req, err := m.msClient.Me().OnlineMeetings().ByOnlineMeetingId(onlineMeetingID).
		Recordings().ByCallRecordingId(recordingID).Content().ToGetRequestInformation(ctx, nil)
	if err != nil {
		return nil, trace.SetCallError(ctx, err)
	}
	// Use raw http client to get the response so we don't need to read the whole response into memory
	rawReq, err := m.msClient.RequestAdapter.ConvertToNativeRequest(ctx, req)
	if err != nil {
		return nil, trace.SetCallError(ctx, err)
	}
	httpReq, ok := rawReq.(*http.Request)
	if !ok {
		return nil, trace.SetCallError(ctx, errors.New("failed to convert to http request"))
	}
	httpReq.URL.Path = nethttplibrary.ReplacePathTokens(httpReq.URL.Path, map[string]string{"/users/me-token-to-replace": "/me"})
	httpClient := &http.Client{
		Timeout: 10 * time.Minute,
	}
	resp, err := httpClient.Do(httpReq)
	if err != nil {
		return nil, trace.SetCallError(ctx, err)
	}
	if resp.StatusCode != http.StatusOK {
		return nil, trace.SetCallError(ctx, fmt.Errorf("failed to get recording content, status code: %d", resp.StatusCode))
	}
	return resp.Body, nil
}