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

feat(generator): separate page for retry policy overrides #11950

Merged
merged 9 commits into from
Jun 23, 2023

Conversation

coryan
Copy link
Contributor

@coryan coryan commented Jun 22, 2023

Part of the work for #10157


This change is Reviewable

@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (3dfe2f2) 93.67% compared to head (9dd3c96) 93.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11950      +/-   ##
==========================================
- Coverage   93.67%   93.66%   -0.01%     
==========================================
  Files        1838     1838              
  Lines      166749   166763      +14     
==========================================
+ Hits       156195   156203       +8     
- Misses      10554    10560       +6     
Impacted Files Coverage Δ
generator/internal/scaffold_generator.cc 63.80% <100.00%> (+0.82%) ⬆️
generator/internal/scaffold_generator_test.cc 100.00% <100.00%> (ø)

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@coryan coryan marked this pull request as ready for review June 22, 2023 22:06
@coryan coryan requested a review from a team as a code owner June 22, 2023 22:06
void GenerateOverrideRetryPoliciesPage(
std::ostream& os, std::map<std::string, std::string> const& variables) {
auto constexpr kText = R"""(/*!
@page $library$-override-retry Override Retry, Backof, and Idempotency Policies
Copy link
Member

Choose a reason for hiding this comment

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

s/Backof/Backoff/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

You can provide your own class for this option. The library also provides two
built-in policies:

- `*LimitedErrorCountRetryPolicy`: stops retrying after an specified number
Copy link
Member

Choose a reason for hiding this comment

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

s/an specified/a specified/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


- `*LimitedErrorCountRetryPolicy`: stops retrying after an specified number
of transient errors.
- `*LimitedTimeRetryPolicy`: stops retrying after an specified time.
Copy link
Member

Choose a reason for hiding this comment

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

s/an specified/a specified/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 876 to 877
Note that a library may have more than one version of these classes. Their name
matches the `*Client` and `*Connection` object they are intended to be used
Copy link
Member

Choose a reason for hiding this comment

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

s/Their name matches/Their names match/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

backoff. The current backoff is doubled after each failure, but never exceeds
(or is "truncated") if it reaches a prescribed maximum.

@section $library$-override-retry-idempotency-policy Controlling the which operations are retryable
Copy link
Member

Choose a reason for hiding this comment

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

s/the which/which/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

GenerateOverrideRetryPoliciesPage(os, vars);
auto const actual = std::move(os).str();
EXPECT_THAT(actual, AllOf(HasSubstr(R"""(
@page test-override-retry Override Retry, Backof, and Idempotency Policies
Copy link
Member

Choose a reason for hiding this comment

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

s/Backof/Backoff/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@coryan
Copy link
Contributor Author

coryan commented Jun 23, 2023

PTAL

@coryan coryan enabled auto-merge (squash) June 23, 2023 01:57
@coryan coryan merged commit 16ecacb into googleapis:main Jun 23, 2023
@coryan coryan deleted the docs-split-override-retry-policy branch June 23, 2023 12:28
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.

2 participants