-
Notifications
You must be signed in to change notification settings - Fork 595
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
Final review for PubSub by Google PubSub team #487
Comments
Yes, the doc is quite a mess actually, looking at the code is essential. Could you please update the doc, the message.id should be message.ackId https://googlecloudplatform.github.io/gcloud-node/#/docs/v0.13.2/pubsub/subscription?method=ack Thank you |
@pierre-b Hi thanks for your concerns. Is there any other specific issues you would like us to address? |
Hi @ryanseys , I noticed a 30sec average lag when I open a subscription and the first message arrives, then the others arrive directly. Is this an expected behavior ? I also need to know how many messages are waiting in the subscription, to pop new workers or monitor the health of my system. Is it in the roadmap actually ? Thank you |
Hi @pierre-b: Can you put these in separate issues so we can track them individually? |
I will look at the code this week. |
Hi @jgeewax , forgot to tell you Ive opened separate issues. |
Pubsub holds the project id, which prevents you from accessing resources in multiple projects. https://github.com/GoogleCloudPlatform/gcloud-node/blob/master/lib/pubsub/index.js#L90 Is this expected? |
Just to be clear, it seems to me that I could do: var gcloud = require('gcloud');
var pubsub1 = gcloud.pubsub({projectId: 'project1'});
var pubsub2 = gcloud.pubsub({projectId: 'project2'}); right? This would allow us to access resources across projects, no? |
@jgeewax I hope you're right. I meant you can not access multiple projects unless creating multiple pubsub object. However, I came up with a bad scenario. I don't think I can access to the subscription2 easily. It seems to me that the only way is to call pubsub.getSubscriptions() and iterate like: var pubsub2 = gcloud.pubsub({projectId: 'project2'});
pubsub2.getSubscriptions(
func(err, subscriptions, nextQuery, apiResponse) {
// I don't come up with how to iterate the result easily
}
) Since if I do the following, the subscription object will have a wrong projectId: var pubsub1 = gcloud.pubsub({projectId: 'project1'});
var topic1 = pubsub1.topic('topic1');
var sub2 = topic1.subscription('subscription2'); // sub2 will have 'project1', which is wrong |
This is the biggest concern from me. Also little bit worried about the performance. I would like to do performance comparison with the google-api-js-client, but never get time to do that. |
Oh, maybe I can do the following? var pubsub2 = gcloud.pubsub({projectId: 'project2'});
var sub2 = pubsub2.subscription('subscription2'); Then, that's fine. |
Apparently, Pubsub object doesn't have subscription method. Can you add it? |
That would just create a reference to an existing subscription, we would also have to support |
Re performance: that is actively not a goal for this right now. P(usability) >> P(performance). That isn't to say that performance will be ignored, but it's lower priority than usability.
Is it reasonable to create a subscription to a topic from a different project? What I read in the docs (https://cloud.google.com/pubsub/reference/rest/v1beta2/projects/subscriptions/create) is that Subscription.create accepts:
If that's the case, without topics being globally unique, is there any possible way to specify a topic from another project? Does "topic" (string) take the form /projects/project-id/topics/topic-name ? |
Yes, it's totally reasonable.
To be precise, it's "projects/project-id/topics/topic-name". It's far better than a situation where resource names are in the global namespace. |
Going to close this out, as I believe we have fixed everything addressed in this issue, and will continue to tackle things one at a time in other issues. Feel free to reopen if I missed something. |
🤖 I have created a release \*beep\* \*boop\* --- ### [2.2.3](https://www.github.com/googleapis/nodejs-dns/compare/v2.2.2...v2.2.3) (2021-09-22) ### Bug Fixes * **deps:** update dependency dns-zonefile to v0.2.9 ([#487](https://www.github.com/googleapis/nodejs-dns/issues/487)) ([53788bd](https://www.github.com/googleapis/nodejs-dns/commit/53788bd5676bf0f5dfd35dde18c912eb98247aa1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Should we have this reviewed more?
The text was updated successfully, but these errors were encountered: