Skip to content

Adds an option to set a timeout for service invocation - #1252

Merged
philliphoff merged 10 commits into
dapr:masterfrom
elena-kolevska:feature/timeout
Apr 8, 2024
Merged

philliphoff merged 10 commits into
dapr:masterfrom
elena-kolevska:feature/timeout

Conversation

@elena-kolevska

@elena-kolevska elena-kolevska commented Mar 18, 2024 •

Copy link
Copy Markdown
Contributor

Description

Adds an option to set a timeout for http and grpc

Issue reference

#1007

Checklist

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
@codecov

codecov Bot commented Mar 18, 2024 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.31%. Comparing base (1b7c9f4) to head (b5962f1).
Report is 1 commits behind head on master.

❗ Current head b5962f1 differs from pull request most recent head c37f86b. Consider uploading reports for the commit c37f86b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1252      +/-   ##
==========================================
+ Coverage   67.28%   67.31%   +0.02%     
==========================================
  Files         174      174              
  Lines        6025     6030       +5     
  Branches      671      672       +1     
==========================================
+ Hits         4054     4059       +5     
  Misses       1802     1802              
  Partials      169      169              
Flag Coverage Δ
net6 67.29% <100.00%> (+0.02%) ⬆️
net7 67.29% <100.00%> (+0.02%) ⬆️
net8 67.29% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena@kolevska.com>
@elena-kolevska
elena-kolevska marked this pull request as ready for review March 18, 2024 23:41
@elena-kolevska
elena-kolevska requested review from a team as code owners March 18, 2024 23:41
…tly to the call as shown in the updated tests and docs

Signed-off-by: Elena Kolevska <elena@kolevska.com>
@elena-kolevska elena-kolevska changed the title Adds an option to set a timeout for http and grpc Adds an option to set a timeout for service invocation Apr 5, 2024
Comment thread src/Dapr.Client/DaprClientBuilder.cs Outdated
// property exposed for testing purposes
internal GrpcChannelOptions GrpcChannelOptions { get; private set; }
internal string DaprApiToken { get; private set; }
internal TimeSpan Timeout { get; private set; } // in milliseconds

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: remove comment? (No longer in milliseconds?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks. Done!

philliphoff
philliphoff previously approved these changes Apr 8, 2024

@philliphoff philliphoff left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just one nit re: comments, otherwise looks good.

Comment thread src/Dapr.Client/DaprClientBuilder.cs Outdated
Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>
@philliphoff
philliphoff merged commit bdca3b3 into dapr:master Apr 8, 2024
@marcduiker

Copy link
Copy Markdown
Contributor

@holopin-bot @elena-kolevska Thanks Elena! 😁

@holopin-bot

holopin-bot Bot commented Apr 24, 2024

Copy link
Copy Markdown

Congratulations @elena-kolevska, you just earned a badge! Here it is: https://holopin.io/claim/clvdvlg1657300gji363rwirt

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

@taufan-mft

Copy link
Copy Markdown

hi guys, first of all thank you for this PR. I've build this version locally and it solved what I needed. Any idea on when this will be included in the release sir @philliphoff ?

@philliphoff

Copy link
Copy Markdown
Collaborator

@taufan-mft It should be part of the v1.14 Dapr release. You can follow its progress in its release planning issue here: dapr/dapr#7605

@taufan-mft

taufan-mft commented Jun 7, 2024 •

Copy link
Copy Markdown

thanks a lot! @philliphoff @elena-kolevska

@philliphoff philliphoff added this to the v1.14 milestone Jul 23, 2024
divzi-p pushed a commit to divzi-p/dotnet-sdk that referenced this pull request Dec 10, 2024
* Adds http timeout

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Adds a timeout for the grpc client

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Small updates

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Updates test

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Adds a timeout example in docs

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Adds e2e test for http service invocation

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Adds tests for grpc service invocation

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Removes grpc timeout, because it’s not needed. It can be passed directly to the call as shown in the updated tests and docs

Signed-off-by: Elena Kolevska <elena@kolevska.com>

* Update src/Dapr.Client/DaprClientBuilder.cs

Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>

---------

Signed-off-by: Elena Kolevska <elena@kolevska.com>
Signed-off-by: Elena Kolevska <elena-kolevska@users.noreply.github.com>
Co-authored-by: Phillip Hoff <phillip@orst.edu>
Signed-off-by: Divya Perumal <diperuma@microsoft.com>
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.

4 participants