Ack. Yeah, I totally agree with some of your points. I'll create a tracking issue to improve the comments and the docs. Thanks a ton for the feedback!
On Monday, April 19, 2021 at 4:10:14 PM UTC-7 kortschak wrote: > Comments in-line. > > On Mon, 2021-04-19 at 15:08 -0700, '[email protected]' via golang-nuts > wrote: > > Ah, glad your issue was resolved. I did want to point out one thing: > > > > > 2. The topic was obtained in the subscriber using new topic > > creation > > > `client.CreateTopic(ctx, topic)` rather than getting a topic > > reference > > > `client.Topic(topic)` which I see now is incorrect (thought which > > is > > > poorly explained in the documentation). > > > > Getting the topic via `create.CreateTopic` or `client.Topic` should > > work > > for subscriber creation. Both return `pubsub.Topic` struct[1], which > > can > > be used to call `topic.Publish` and pass to subscription config. > > I quickly tested this to confirm, code here[2]. > > If I make this change to github.com/kortschak/scheduler > ``` > index 6f62da8..8f3fb87 100644 > --- a/listener/main.go > +++ b/listener/main.go > @@ -133,7 +133,11 @@ topics should be subscribed to using the default > subscription config. > log.Printf("using default config: %v", cfg.DefaultConfig) > subConfig = cfg.DefaultConfig > } > - subConfig.Topic = client.Topic(sub.Topic) > + t, err := client.CreateTopic(ctx, sub.Topic) > + if err != nil { > + log.Fatalf("failed to create topic %q: %v", sub.Topic, err) > + } > + subConfig.Topic = t > s, err := client.CreateSubscription(ctx, sub.ID, subConfig) > if err != nil { > if grpc.Code(err) == codes.AlreadyExists { > ``` > > I see this error when I run scheduler and listener. > > ``` > 2021/04/20 08:02:58 available topics: > 2021/04/20 08:02:58 projects/testing/topics/cron-job > 2021/04/20 08:02:58 projects/testing/topics/cron-job-again > 2021/04/20 08:02:58 subscribing to "cron-job" as "test-0" > 2021/04/20 08:02:58 failed to create topic "cron-job": rpc error: code > = AlreadyExists desc = Topic already exists > exit status 1 > ``` > The issue here is that the topic has already been created by the > publisher. This is where greater contextual information in the docs > would be helpful. My reading of the docs for *Client.Topic, "Topic > creates a reference to a topic in the client's project", was that there > would already have to be a *pubsub.Topic and that that would be *local* > (this last bit is not true — it was influenced by notions of file > references). Unfortunately the terseness of the docs don't help here. > > > > Lastly, with regards to the cmdline examples, I'm guessing this was > > removed because we had the official gcloud sdk `gcloud pubsub ...`. > > Although the gcloud sdk doesn't use the Go client, I think we didn't > > think it would be necessary to support two API surfaces. > > My suggestion was not for use as a client, but as an integrated > example. > > > > We do keep our code samples in our golang-samples[3] repo, > > and I'm not sure if it was these samples that you found were > > hard to navigate, > > It's not that they are hard to navigate, the docs in > https://cloud.google.com/pubsub/docs/samples is very navigable, it's > just that the scale of the context for them is not very useful being > only slightly larger scale than a reader would get from the godoc. The > context could be improved either by reducing the terseness of the godoc > (for example saying for *Client.CreateTopic that *Client.Topic should > be used if the *Topic has already been created), or by providing actual > runnable examples in the form of example tests, which pkg.go.dev does a > reasonably good job of turning into main packages. > > > > but if so, we're very much open to feedback. I think I can sort of > > see your point of how the cmdline example is easier to read (with it > > being all in one file), but it's part of the repo's style to keep > > code samples in separate files. > > It's not so much that they are separate files, but rather that there is > little contextual information showing how they should be composed. > > > > For learning, I'll work on adding a more comprehensive end-to-end > > guide that lives in the package docs instead. > > Yes, I think this would be a good way to go. It doesn't need to be > extensive, just hitting a reasonable set of common compositions and > uses. > > thanks > Dan > > > [1] https://pkg.go.dev/cloud.google.com/go/pubsub#Topic > > [2] https://play.golang.org/p/Vpe6RxP6Db3 > > [3] > > https://github.com/GoogleCloudPlatform/golang-samples/tree/master/pubsub > > > > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/f09b1f2c-5736-4ede-bf5e-6546db2f676en%40googlegroups.com.
