-
Notifications
You must be signed in to change notification settings - Fork 92
docs(pubsub): add user guide quickstart for Pub/Sub #3764
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3764 +/- ##
=======================================
Coverage 96.14% 96.14%
=======================================
Files 140 140
Lines 5442 5442
=======================================
Hits 5232 5232
Misses 210 210 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
coryan
left a comment
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.
Lots of nits, I am not sure if they are useful. We need to decide if we use cloud.google.com directly for these. Also need to decide if the quickstart only includes one service (I think it should).
| // ANCHOR: topicadmin | ||
| use google_cloud_pubsub::client::TopicAdmin; | ||
| let topic_admin = TopicAdmin::builder().build().await?; | ||
| // ANCHOR_END: topicadmin | ||
|
|
||
| // ANCHOR: createtopic | ||
| let topic = topic_admin | ||
| .create_topic() | ||
| .set_name(format!("projects/{project_id}/topics/{topic_id}")) | ||
| .send() | ||
| .await?; | ||
| // ANCHOR_END: createtopic |
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.
Mixing admin and data operations in the same "quickstart" seems more confusing than it needs to be. Could we split this?
| let mut handles = Vec::new(); | ||
| for i in 0..10 { | ||
| use google_cloud_pubsub::model::PubsubMessage; | ||
| let msg = PubsubMessage::new().set_data(format!("message {}", i)); | ||
| handles.push(publisher.publish(msg)); | ||
| } |
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.
Too clever?
| let mut handles = Vec::new(); | |
| for i in 0..10 { | |
| use google_cloud_pubsub::model::PubsubMessage; | |
| let msg = PubsubMessage::new().set_data(format!("message {}", i)); | |
| handles.push(publisher.publish(msg)); | |
| } | |
| let handles = (0..10).iter().map(|i| { | |
| use google_cloud_pubsub::model::PubsubMessage; | |
| let msg = PubsubMessage::new().set_data(format!("message {}", i)); | |
| handles.push(publisher.publish(msg)) | |
| }).collect::<Vec<_>>(); |
| #[tokio::test(flavor = "multi_thread")] | ||
| async fn quickstart() -> anyhow::Result<()> { | ||
| let project_id = std::env::var("GOOGLE_CLOUD_PROJECT").unwrap(); | ||
| let topic_id = random_topic_id(); |
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.
Does the Pub/Sub integration test garbage collect any topics leaked here? Can you add a comment?
| let result = quickstart::quickstart(&project_id, &topic_id).await; | ||
| result |
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.
If you move the topic creation and clean up to this file then:
| let result = quickstart::quickstart(&project_id, &topic_id).await; | |
| result | |
| let result = quickstart::quickstart(&project_id, &topic_id).await; | |
| cleanup_topic(...); | |
| result |
Otherwise:
| let result = quickstart::quickstart(&project_id, &topic_id).await; | |
| result | |
| quickstart::quickstart(&project_id, &topic_id).await |
| // limitations under the License. | ||
|
|
||
| // ANCHOR: quickstart | ||
| pub async fn quickstart(project_id: &str, topic_id: &str) -> anyhow::Result<()> { |
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.
I started to call all these functions "sample" so I don't have to think.
|
|
||
| ## Creating a topic | ||
|
|
||
| The client to perform operations on topics is called `TopicAdmin`: |
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.
Maybe:
| The client to perform operations on topics is called `TopicAdmin`: | |
| To perform operations on topics use the `TopicAdmin` client: |
| ## Publishing a Message | ||
|
|
||
| `google-cloud-pubsub` exposes a high-level client that can batch messages | ||
| together for a given topic. To create these batched `Publisher`s, first create a |
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.
Maybe add something like: "... given topic. Batching messages results in better performance with high throughput clients, as multiple messages are sent in a single RPC."
| `google-cloud-pubsub` exposes a high-level client that can batch messages | ||
| together for a given topic. To create these batched `Publisher`s, first create a | ||
| `PublisherFactory`, which configures and manages low-level gRPC client and use | ||
| it to create a `Publisher`. |
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.
the ... which configures is too much detail for a tutorial. That is more a topic for in-depth guides where you would discuss optimizations and dive into the implementation details.
| {{#rustdoc_include ../samples/tests/pubsub/quickstart.rs:publisher}} | ||
| ``` | ||
|
|
||
| We can then publish messages to the topic. These messages are batched in a |
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.
The style guide (again) suggests using "you" over "we", and prefer "Publish some messages to the topic. The client library batches these messages in the background, before sending them to the service:"
|
|
||
| ### Cleanup | ||
|
|
||
| Finally we remove the topic to cleanup all the resources used in this guide: |
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.
The style guide things.
Adds an initial user guide for the Pub/Sub client library.
This is more or less the same example that we have in the google_cloud_pubsub::client documentation.