feat(pubsub): include subscription name in StreamingPull header#4585
feat(pubsub): include subscription name in StreamingPull header#4585haphungw wants to merge 1 commit intogoogleapis:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4585 +/- ##
==========================================
+ Coverage 95.01% 95.02% +0.01%
==========================================
Files 195 195
Lines 7456 7459 +3
==========================================
+ Hits 7084 7088 +4
+ Misses 372 371 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dbolduc
left a comment
There was a problem hiding this comment.
How can we test this so the code doesn't regress?
I have an idea (SPOILERS)
We have a mock Pub/Sub service ...
... which we use in the session.rs unit tests. Maybe we can add a new unit test where we extract the metadata from the incoming tonic::Request and check that it has the header we just set?
https://docs.rs/tonic/0.14.2/tonic/struct.Request.html#method.metadata
| type Stream = MockStream; | ||
| async fn streaming_pull( | ||
| &self, | ||
| subscription: &str, |
There was a problem hiding this comment.
comment: This works, and I am ok with it.
Some background: this is a handwritten interface for a bidirectional gRPC stream added onto a fully generated struct.
We have not come up with a generic solution for bidirectional streams yet that we can generate.
So subscription works but it won't help us solve the general problem. (Which might look like accepting an initial request? (although I don't think in general that is even known. Ugh. It is a hard problem that probably @coryan will have to tackle next quarter)).
There was a problem hiding this comment.
Consider accepting the whole x_goog_request_params
Fix #4470