feat(pubsub): add simple Publisher builder#4343
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4343 +/- ##
==========================================
+ Coverage 94.98% 95.02% +0.04%
==========================================
Files 193 194 +1
Lines 7294 7359 +65
==========================================
+ Hits 6928 6993 +65
Misses 366 366 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7efb993 to
b2cd951
Compare
dbolduc
left a comment
There was a problem hiding this comment.
Can you fix the examples to use the right client?
And can you also do a pass on the coverage for some of the new options? #4343 (comment)
The nits around wording are not a blocker for me approving
b2cd951 to
400c9fd
Compare
suzmue
left a comment
There was a problem hiding this comment.
Need to refactor to test the Client setters effectively, since I can't inspect anything in the ClientBuilder
Ah. I thought for like ~10 mins and didn't come up with a simple, obvious refactor. Maybe write your own |
8c7618c to
c2e8673
Compare
Doing this made me sad, but also its a lot better to be able to verify the configs match so this is an improvement. TY for pushing for this |
dbolduc
left a comment
There was a problem hiding this comment.
Some nits to consider in a follow up.
TY for pushing for this
Hopefully I was not too pushy. FTR, I would have approved a PR without coverage if there was a plan to add coverage later.
| } | ||
| } | ||
| } | ||
| pub use super::client_builder::ClientBuilder as BasePublisherBuilder; |
There was a problem hiding this comment.
nit: seems less confusing to name the internal class BasePublisherBuilder
| /// Publishers are created via a [`BasePublisher`][crate::client::BasePublisher]. | ||
| /// A builder for a `Publisher`. | ||
| #[derive(Clone, Debug)] | ||
| pub struct PublisherBuilder { |
There was a problem hiding this comment.
organization nit: does this belong in client_builder.rs now?
Or maybe it would be clearest with:
publisher_builder.rsbase_publisher_builder.rs
Add a new builder that can configure both the client settings and the batching settings for the Publisher and updated the examples to use the new builder.
This change also refactors the BasePublisherBuilder to be its own type to allow for more visibility for testing.
The doc comments and examples are copied from the ClientBuilder and the PublisherPartialBuilder. The only client configuration that is not included is the polling configuration since this isn't relevant.
For #4282