Skip to content

Feat: Adds batch requests - #111

Merged
rkodev merged 22 commits into
mainfrom
rkodev/batch-requests
Oct 5, 2022
Merged

Feat: Adds batch requests#111
rkodev merged 22 commits into
mainfrom
rkodev/batch-requests

Conversation

@rkodev

@rkodev rkodev commented Aug 24, 2022

Copy link
Copy Markdown
Contributor

Overview

Adds BatchRequest

Demo

reqInfo := getRequestInfo()
batch := NewBatchRequest()
batch.AppendBatchItem(*reqInfo)

resp, err := SendBatch(*batch, reqAdapter)

Notes

I used a no-op Parsable struct to bypass SendAsync's type assertion. We should consider splitting what SendAsync does into two parts. One that just gets the raw response and a second part that does the type assertion. Otherwise, there's no simple way for users to get raw responses.

Testing

  • cd to project root
  • Run go test

Closes #11

Address comments on #71
Closes #71

@baywet baywet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up, a few initial thoughts

Comment thread docs/batch_request.md Outdated
Comment thread docs/batch_request.md Outdated
Comment thread docs/batch_request.md Outdated
resp, err := msgraphsdkcore.SendBatch(*batch, reqAdapter)

// print the first response
fmt.Println(resp.Responses[0].Body)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

additionally the response or the model should be retrieved using the batch item id and not the array index to avoid any kind of confusion, misordering: the service doesn't guarantee the responses will be in the same order as the requests.

Comment thread batch_requests.go Outdated
Comment thread batch_requests.go Outdated
@rkodev
rkodev force-pushed the rkodev/batch-requests branch from 2f5d19c to 2cde1e3 Compare September 29, 2022 06:21

@baywet baywet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great progress, I think we're almost there besides a few minor comments.

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md
Comment thread batch_item_model.go
Comment thread batch_requests.go Outdated
@sonarqubecloud

sonarqubecloud Bot commented Oct 4, 2022

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

81.0% 81.0% Coverage
0.0% 0.0% Duplication

@rkodev
rkodev merged commit 72adea9 into main Oct 5, 2022
@rkodev
rkodev deleted the rkodev/batch-requests branch October 5, 2022 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for batch requests

2 participants