Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Set a context timeout when calling Graph SDK #2849

Closed
vkamra opened this issue Mar 17, 2023 · 4 comments
Closed

Set a context timeout when calling Graph SDK #2849

vkamra opened this issue Mar 17, 2023 · 4 comments
Assignees
Labels
bug Something isn't working graph api for changes related to communicating directly with the graph rest api p0

Comments

@vkamra
Copy link
Contributor

vkamra commented Mar 17, 2023

The Graph SDK will set a 100s timeout on the context if a deadline is not set on the provided ctx.

This is not-sufficient for long running requests (e.g. Attachment fetch) and also is not being reset across retry attempts.

We need to set an appropriate timeout when calling the SDK (we currently use ctx.Background() which triggers the "no deadline" logic in the SDK). Also need to make sure this timeout is reset for retries.

@vkamra vkamra added bug Something isn't working graph api for changes related to communicating directly with the graph rest api p0 labels Mar 17, 2023
@meain
Copy link
Member

meain commented Mar 20, 2023

Fixed upstream via microsoft/kiota-http-go#71 .

@meain meain mentioned this issue Mar 20, 2023
12 tasks
@meain meain self-assigned this Mar 20, 2023
@meain
Copy link
Member

meain commented Mar 21, 2023

Still have to deal with #2875 (comment)

aviator-app bot pushed a commit that referenced this issue Mar 22, 2023
This has been fixed upstream with microsoft/kiota-http-go#71 . We don't need this hack anymore.

---

#### Does this PR need a docs update or release note?

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #2849
* #2694
* closes #2893
* closes #2892
* #2891

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [ ] ⚡ Unit test
- [ ] 💚 E2E
@meain
Copy link
Member

meain commented Mar 28, 2023

@meain
Copy link
Member

meain commented Mar 30, 2023

Closing this as the original issue is fixed. Opened #2984 to track infinite timeouts.

@meain meain closed this as completed Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working graph api for changes related to communicating directly with the graph rest api p0
Projects
None yet
Development

No branches or pull requests

2 participants