Skip to content
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

Default context deadline to client timeout #71

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

rkodev
Copy link
Contributor

@rkodev rkodev commented Mar 20, 2023

This change syncronizes the deafult context deadline and the client Timeout. When a user does not pass a context deadline, the default fallback will not be the value of the client timeout. This should also make it possible to modify default context deadlines for a single point addressing #70

Should also prevent the client timeout overriding the context timeout when its significantly lower microsoftgraph/msgraph-sdk-go#302 (comment)

To modify a deadline, you can pass initialize NetHttpRequestAdapter with a modified client.Timeout property.

client := GetDefaultClient()
client.Timeout = time.Second * 20

adapter := NewNetHttpRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient(provider, nil, nil, client)

```

@sonarcloud
Copy link

sonarcloud bot commented Mar 20, 2023

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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@baywet baywet linked an issue Mar 20, 2023 that may be closed by this pull request
@rkodev rkodev merged commit 5926d58 into main Mar 20, 2023
@rkodev rkodev deleted the feat/default-context-deadline-to-client branch March 20, 2023 14:34
aviator-app bot pushed a commit to alcionai/corso that referenced this pull request 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there some way to globally override default 100s timeout
2 participants