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.

Sign up for GitHub

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

Jump to bottom

Add transient fault handling doc for gRPC retries #21621

Merged
merged 19 commits into from
Mar 9, 2021
Merged

Add transient fault handling doc for gRPC retries #21621

merged 19 commits into from
Mar 9, 2021

Conversation

JamesNK
Copy link
Member

@JamesNK JamesNK commented Feb 25, 2021 •
edited
Loading

@wadepickett wadepickett self-requested a review February 25, 2021 17:45
@wadepickett
Copy link
Contributor

wadepickett commented Feb 25, 2021 •
edited
Loading

@JamesNK, do you want me to do a review to get that started? You labeled it blocked I assume since we all can't get builds to look at yet and that may be down for a few days yet, but I could help move it along in between time anyway until we can see a build.

@JamesNK
Copy link
Member Author

JamesNK commented Feb 25, 2021

Yes, review now. The content is done. The block is so it doesn’t get merged.

wadepickett
wadepickett requested changes Feb 25, 2021
View reviewed changes
Copy link
Contributor

@wadepickett wadepickett left a comment

Choose a reason for hiding this comment

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

The doc looks good. All I found were minor nitpicky things. Thanks for letting me review James.

aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
JamesNK and others added 2 commits February 26, 2021 09:40
@JamesNK @wadepickett
Apply suggestions from code review
738264f 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
@JamesNK @wadepickett
Update aspnetcore/grpc/retries.md
aa08a14 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
JamesNK
JamesNK commented Feb 25, 2021
View reviewed changes
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
JamesNK
JamesNK commented Feb 25, 2021
View reviewed changes
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
@wadepickett
Copy link
Contributor

..Waiting for the build system to come back online, then I will verify the doc in the build and squash and merge.

@JamesNK
Copy link
Member Author

JamesNK commented Feb 26, 2021

This shouldn’t be merged until the linked PR is merged and released. That will be in a weeks time.

Base automatically changed from master to main March 8, 2021 16:32
@wadepickett
Copy link
Contributor

@JamesNK I saw the related PR was merged. Is this one still in flux or should I review changes now?

@JamesNK
Copy link
Member Author

JamesNK commented Mar 9, 2021

Go ahead. Will probably merge this later today.

@JamesNK JamesNK removed the blocked label Mar 9, 2021
wadepickett
wadepickett requested changes Mar 9, 2021
View reviewed changes
Copy link
Contributor

@wadepickett wadepickett left a comment

Choose a reason for hiding this comment

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

@JamesNK , Looks good. Just some very minor items for you to consider to commit. Thanks!

aspnetcore/grpc/configuration.md Outdated Show resolved Hide resolved
aspnetcore/grpc/configuration.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
aspnetcore/grpc/retries.md Show resolved Hide resolved
aspnetcore/grpc/retries.md Show resolved Hide resolved
aspnetcore/grpc/retries.md Outdated Show resolved Hide resolved
JamesNK and others added 3 commits March 10, 2021 09:31
@JamesNK @wadepickett
Update aspnetcore/grpc/configuration.md
cf243d5 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
@JamesNK @wadepickett
Update aspnetcore/grpc/configuration.md
ea9e8bd 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
@JamesNK @wadepickett
Update aspnetcore/grpc/retries.md
007aeb9 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
JamesNK and others added 4 commits March 10, 2021 09:34
@JamesNK @wadepickett
Update aspnetcore/grpc/retries.md
5b75049 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
@JamesNK @wadepickett
Update aspnetcore/grpc/retries.md
1280358 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
@JamesNK @wadepickett
Update aspnetcore/grpc/retries.md
51d91e4 
Co-authored-by: Wade Pickett <wpickett@microsoft.com>
@JamesNK
Minor
6795ba5
@JamesNK JamesNK requested a review from wadepickett March 9, 2021 20:40
wadepickett
wadepickett requested changes Mar 9, 2021
View reviewed changes
aspnetcore/toc.yml Show resolved Hide resolved
@wadepickett wadepickett merged commit 4955eeb into main Mar 9, 2021
@wadepickett wadepickett deleted the jamesnk/grpc-retries branch March 9, 2021 21:00
@scottaddie scottaddie changed the title Add gRPC retries doc Add transient fault handling doc for gRPC retries Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wadepickett wadepickett wadepickett approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JamesNK @wadepickett