-
Notifications
You must be signed in to change notification settings - Fork 388
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
fix(generator): Connection base-class operations should fail #8236
Conversation
While we do not test the behavior of the generated "ServiceConnection" base classes, all operations should fail with `kUnimplemented`. That was not the case for paginated calls, which previously returned an empty sequence instead.
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov Report
@@ Coverage Diff @@
## main #8236 +/- ##
==========================================
+ Coverage 94.95% 94.97% +0.01%
==========================================
Files 1327 1327
Lines 119194 119169 -25
==========================================
- Hits 113186 113185 -1
+ Misses 6008 5984 -24
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we return this instead:
return google::cloud::internal::MakeUnimplementedPaginationRange<
StreamRange<$range_output_type$>>();
google-cloud-cpp/google/cloud/internal/pagination_range.h
Lines 182 to 189 in 306d12b
template <typename Range> | |
Range MakeUnimplementedPaginationRange() { | |
using ValueType = typename Range::value_type::value_type; | |
return MakeStreamRange<ValueType>( | |
[]() -> typename StreamReader<ValueType>::result_type { | |
return Status{StatusCode::kUnimplemented, "needs-override"}; | |
}); | |
} |
where maybe we change the message to "not implemented" instead of "needs-override"
While we do not test the behavior of the generated "ServiceConnection" base classes, all operations should fail with kUnimplemented. That was not the case for paginated calls, which previously returned an empty sequence instead.
Done. PTAL. |
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Note: Unfortunately the change in google/cloud/internal/pagination_range.h from "needs-override" to "not implemented" was incorrectly grouped in the "regeneration" commit ef7628a. |
While we do not test the behavior of the generated "ServiceConnection"
base classes, all operations should fail with
kUnimplemented
. Thatwas not the case for paginated calls, which previously returned an
empty sequence instead.
This change is