Comments in-line.

On Mon, 2021-04-19 at 15:08 -0700, 'hong...@google.com' 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 golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/b610b7eb25b43bc572a8f67162ada4b0c8cef73e.camel%40kortschak.io.

Reply via email to