msgraph-sdk-dotnet-core icon indicating copy to clipboard operation
msgraph-sdk-dotnet-core copied to clipboard

Add real batch support

Open habex-ch opened this issue 5 years ago • 9 comments

The current implementation for the batch-requests is a nice helper for writting my own HttpClient-Batch-Request, but is far of a real batch support as it was descriped by @darrelmiller in https://github.com/microsoftgraph/msgraph-sdk-dotnet/issues/136#issuecomment-411784419.

For example I have to add 600 users to a group (https://docs.microsoft.com/en-us/graph/api/group-post-members). Unfortunately, there is no API, which supports this. So I have to send 600 requests.

With the current batch-request implementation I have to

  1. Create a JObject for each new member: (new JObject { { "@odata.id", $"https://graph.microsoft.com/v1.0/directoryObjects/{memberId}" } })
  2. Create my own HttpRequestMessage objects
  3. Add my HttpRequestMessage objects to BatchRequestSteps
  4. Bundle those BatchRequestSteps into multiple BatchRequestContent (because only 20 requests per BatchRequestContent are allowed)
  5. Create for each BatchRequestContent another HttpRequestMessage
  6. Create an HttpClient (don't forget authentication)
  7. Send the HttpRequestMessages of the batch-requests
  8. Get the BatchResponseContent from the HttpResponseMessage
  9. Get the HttpResponseMessage for each single request
  10. Interpret the responses
  11. And so on

I would expect something like @darrelmiller has described:

string groupId = ...
IList<DirectoryObject> newMembers = ...
var batch= new BatchRequestContent();
foreach (var newMember in newMembers)
{
    batch.Add(graphClient.Groups[groupId].Members.References.Request().Add(newMember))
}
var results = graphClient.Batch(batch).Request().ExecuteAsync();

Considerations: A) batch.Add() could have a second parameter and third parameter for a specific "requestId" and "dependsOn". Like Add<TEntity>(TEntity, string requestId = null, string[] dependsOn = null) If none of them are set, the BatchRequestContent should set its own requestId (e.g. a counter++)

B) The results should be in the same order as the request were.

C) If there are more than 20 Requests, the batch should handle them itself

D) If there would be response-content of e.g. type "User", the results could be interpreted like this:

...
if(results.IsSuccessStatusCode)
{
    foreach(var result = batch.Items)
    {
        try
        {
            //result.Get<>() should call throw an exception as graphClient.Groups[groupId].Members.References.Request().AddAsync(newMember);
            User addedUser = result.Get<User>(); 
        }
        catch(Exception ex)
        {
            Logger.LogError(ex, "Couldn't add user to group");
        }
    }
}

AB#7195

habex-ch avatar Jul 11 '19 17:07 habex-ch

Hey @habex-ch

Thanks for the feedback. Sorry we didn't quite hit the mark but I think we can get much closer than what you are current describing. By calling .Request().GetHttpRequestMessage() you should be able to bypass the manual building of the HttpRequestMessage. We are missing something to be able to specify the body though.

I wonder if we could enhance requestBuilders to hold onto bodies. So we could do

graphClient.Groups[groupId].Members.References.WithMember(newMember).Request().GetHttpRequestMessage()

We can't control the order that we get the responses, but we could offer the ability to return the responses in the order of the requestId which would help if requestId was created in order.

Option C is a brilliant idea that fits so well with some of the other experiments we are doing. I love it. This is going to happen for sure.

You should be able to get close to the last item using our responseHandler.HandleResponse<User>(response). We can probably make it cleaner though.

We will take your scenario and try and address the issues you raise with what we currently have and see where the gaps are that we need to fill.

Thanks for your feedback.

darrelmiller avatar Jul 17 '19 12:07 darrelmiller

Thank you for reply.

Thanks for the feedback. Sorry we didn't quite hit the mark but I think we can get much closer than what you are current describing. By calling .Request().GetHttpRequestMessage() you should be able to bypass the manual building of the HttpRequestMessage. We are missing something to be able to specify the body though.

It only helps to create an instance of a HttpRequestMessage, but I still have to

  1. Change the method to "Post
  2. Create the content as a jsonString and add it

I wonder if we could enhance requestBuilders to hold onto bodies. So we could do

graphClient.Groups[groupId].Members.References.WithMember(newMember).Request().GetHttpRequestMessage()

It would help, but I would still have to set the method to something different. I think better would be a method like graphClient.Groups[groupId].Members.References.Request().GetHttpRequestMessageForAdd(newMember); So the syntax would almost be the same as the direkt usage.

Maybe a whole change of the syntax like graphClient.Groups[groupId].Members.References.Request().Add(newMember).ExecuteAsync(); instead of .AddAsync(newMember) should be a consideration. So .Add(newMember) could be used for the batch scenario.

We can't control the order that we get the responses, but we could offer the ability to return the responses in the order of the requestId which would help if requestId was created in order.

That would be great 👍

Option C is a brilliant idea that fits so well with some of the other experiments we are doing. I love it. This is going to happen for sure.

Perfect 👍 .

You should be able to get close to the last item using our responseHandler.HandleResponse(response). We can probably make it cleaner though.

I totally missed that feature. Thank you for the hint.

We will take your scenario and try and address the issues you raise with what we currently have and see where the gaps are that we need to fill.

I am looking forward for a good batch solution :-)

habex-ch avatar Jul 18 '19 09:07 habex-ch

Thanks for your feedback. I hadn't properly re-read my original suggestion. How would you feel about this?

graphClient.Groups[groupId].Members.References.AsPost(newMember).Request().GetHttpRequestMessage()

darrelmiller avatar Jul 18 '19 12:07 darrelmiller

That would assume, that the programmer knows which kind of method (get/post/patch/...) it is. Whereas he doesn't need that information in any other part of the SDK.

Is there always only 1 object, or can there sometimes be multiple objects?

What's wrong with graphClient.Groups[groupId].Members.References.Request().Add(newMember).GetHttpRequestMessage() or how about graphClient.Groups[groupId].Members.References.Request().PrepareAdd(newMember).GetHttpRequestMessage() or graphClient.Groups[groupId].Members.References.AsAdd(newMember).Request().GetHttpRequestMessage() if it has to be before the "Request()" method?

habex-ch avatar Jul 18 '19 21:07 habex-ch

It doesn't have to be before the Request(), but I just want to find the most natural syntax.

darrelmiller avatar Aug 29 '19 18:08 darrelmiller

I think most natural would be graphClient.Groups[groupId].Members.References.Request().Add(newMember).GetHttpRequestMessage()

habex-ch avatar Sep 04 '19 19:09 habex-ch

I am new to graph apis and looking for creating group calendar events in bulk (batch) using graph apis. Is there a workaround to achieve something like this?

_graphServiceClient.Groups[userGroupId].Events.Request().AddAsync(@eventList);

praveenlearns avatar Nov 06 '19 05:11 praveenlearns

@praveenlearns I have described the current batch implementation in my first post. Did you try that?

habex-ch avatar Nov 06 '19 07:11 habex-ch

Created an issue to address the ability to create Requests with a body to add to batches. https://github.com/microsoftgraph/MSGraph-SDK-Code-Generator/issues/229

darrelmiller avatar Dec 03 '19 22:12 darrelmiller

Hello everyone.

As part of the new core v3 version, we do have the ability of creating requests with a body to add to batches. This can be accomplished by using RequestInformation as the following:

var graphClient = new GraphServiceClient(...);

// Create a BatchRequestContent
var batchRequestContent = new BatchRequestContent(graphClient);

// Add requests to it, max of 20
var usersRequest= await batchRequestContent.AddBatchRequestStepAsync(graphClient.Me.ToGetRequestInformation());
var eventsRequest = graphClient.Me.CalendarView
    .ToGetRequestInformation(requestConfiguration => 
        {
            requestConfiguration.QueryParameters.StartDateTime = today.ToString("yyyy-MM-ddTHH:mm:ssK");
            requestConfiguration.QueryParameters.EndDateTime = today.AddDays(1).ToString("yyyy-MM-ddTHH:mm:ssK");
        });

//Using AddBatchRequestStepAsync adds each request as a step
//No specified order of execution
var userRequestId = await batchRequestContent.AddBatchRequestStepAsync(userRequest);
var eventsRequestId = await batchRequestContent.AddBatchRequestStepAsync(eventsRequest);

// Execute the batch request
var response = await graphClient.Batch.PostAsync(batchRequestContent);

// De-serialize response based on known return type
try
{
    var user = await returnedResponse
        .GetResponseByIdAsync<User>(userRequestId);
    Console.WriteLine($"Hello {user.DisplayName}!");
}
catch (ServiceException ex)
{
    Console.WriteLine($"Get user failed: {ex.Error.Message}");
}

This is pretty similar to what was described by Darrel in the comment mentioned in the description of this issue. We are closing this as we believe the initial request has been reached.

The next step is to evaluate improving batching support to handle the limit of 20 requests tracked by https://github.com/microsoftgraph/msgraph-sdk-dotnet-core/issues/612

maisarissi avatar Mar 03 '23 17:03 maisarissi

@habex-ch I got you covered, adding 400 users to a group in 1 http call... Want to give it a go?

https://svrooij.io/2023/03/03/batching-in-microsoft-graph/#adding-400-users-to-a-group

svrooij avatar Mar 03 '23 22:03 svrooij