Skip to content

Add support for shutdown API and add client docs#922

Merged
halspang merged 2 commits into
dapr:masterfrom
halspang:shutdown_api
Aug 23, 2022
Merged

Add support for shutdown API and add client docs#922
halspang merged 2 commits into
dapr:masterfrom
halspang:shutdown_api

Conversation

@halspang
Copy link
Copy Markdown
Contributor

Description

This commit adds support for the shutdown API. It also adds docs for
that method and a few others which were missing from the docs.

#914

Signed-off-by: Hal Spang halspang@microsoft.com

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #914

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

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

@halspang halspang requested review from a team as code owners August 10, 2022 00:46
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 10, 2022

Codecov Report

Merging #922 (81a44b9) into master (e6ded69) will decrease coverage by 0.04%.
The diff coverage is 90.47%.

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
- Coverage   69.68%   69.63%   -0.05%     
==========================================
  Files         155      155              
  Lines        5119     5121       +2     
  Branches      553      553              
==========================================
- Hits         3567     3566       -1     
- Misses       1421     1424       +3     
  Partials      131      131              
Flag Coverage Δ
net5 ?
net6 ?
netcoreapp3.1 69.63% <90.47%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
src/Dapr.Client/DaprClient.cs 93.61% <ø> (ø)
src/Dapr.Client/DaprClientGrpc.cs 86.61% <90.47%> (-0.12%) ⬇️
src/Dapr.Actors/DaprHttpInteractor.cs 53.59% <0.00%> (-1.11%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment thread src/Dapr.Client/DaprClient.cs Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpicking:

Do we generally try to keep the func/method name the same across SDKs?

I'm asking this because on go-sdk and python-sdk we named it as Shutdown rather than ShutdownSidecar[Async]

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.

This is something that we'll define in the sig-sdk group, but I do think it's important to keep naming consistent. I do worry that both of those are a little to lax on the name though.

Do you think it's worth being different here for clarity? Having the API just be Shutdown, to me, doesn't really tell the user what it's doing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't have a strong opinion here,

There are negative and positive effects of being explicit in this case, I do not feel much comfortable exposing the word Sidecar I would say that "ShutdownDaprd" sound better, and, if by any chance we would have more than one sidecar (probably won't happen) it becomes explicit which one is shutting down. But again, it's nitpicking and I agree with you that just Shutdown is too generic

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 to everything said here. This seems like a good chance to be consistent on the terminology.

Comment thread src/Dapr.Client/DaprClientGrpc.cs Outdated
Comment thread src/Dapr.Client/DaprClientGrpc.cs Outdated
This commit adds support for the shutdown API. It also adds docs for
that method and a few others which were missing from the docs.

dapr#914

Signed-off-by: Hal Spang <halspang@microsoft.com>
await client.ShutdownAsync(new Empty(), CreateCallOptions(null, cancellationToken));
}

#endregion
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pretty simple :)

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.

No option to shut down the side car

3 participants