-
Notifications
You must be signed in to change notification settings - Fork 592
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
pubsub: API refactor #98
Comments
Looks generally good. Two issues:
|
I was thinking that would happen with the initial
I forgot to annotate what I did with that. Both of these bits would create a subscription: // Subscribe to a Topic (simple [auto subscription name generated])
var puppyLogger = PuppiesTopic.subscribe({
autoAck: true,
ackDeadlineSeconds: 60
});
// Subscribe to a Topic (specific [subscription name specified])
var puppyLogger = PuppiesTopic.subscribe({
name: 'puppyLogger',
autoAck: true,
ackDeadlineSeconds: 60
}); The first one would generate a subscription name for you, but if you wanted to provide one, simply provide a "name" property in the configuration object. I'm not sure if that's a great idea, definitely need more thoughts on that. My thought was, I may not care about naming it, as I never intend to refer to it again from another connection. If I do need to, I can provide a name. Is that a common enough scenario, or should we require user specification of a name?
|
I filed an issue against API that name should be generated if not provided, and team said my assumption about subscriptions being transient is wrong. In the typical scenario, a user will create a set of topics and subscriptions via a script before deploying their app. They will embed the topic and subscription names to their app config and will never run CRUD operations on topics and subscriptions from the application context. |
b/16462864 |
No problemo. Updated the first post with these changes. |
Looks great! |
@stephenplusplus, I think we can start working on this issue. Would you like to focus on it? |
Yep! I actually have a good start, just fine tuning and writing tests On Monday, August 11, 2014, Burcu Dogan notifications@github.com wrote:
|
Great! |
Opening this issue again, we still need to improve the API. |
Have 2 questions. // Subscribe to a Topic (specific [subscription name specified]) In my understanding, the above code will create a new subscription if not exist. If the subscription with that name exists, what will happen? What if the existing subscription has a push endpoint (push subscription)? Why the connection needs projectId? Potentially Pub/Sub can be used for building a kind of system dashboard across multiple projects. In that case, you need to create as much connections as project numbers with the current lib. |
Generally I like the natural taste of the suggested API. Do you have any plan for providing a similar shortcut for push subscriptions? |
I think you meant push notifications. There is currently no plan for an out of the box solution, because it involves adding a handler to user's existing web server code. We can provide a middleware for that, but currently not in our scope. The rest of the questions above is what we should address once we get back working on the API improvements. |
I've updated the first post with a new proposal and simple example usages. I liked the other approach as well (at the bottom of the first post), but there was too much magic involved that could be confusing. Please let me know your thoughts! |
Does pubsub.getTopic() invoke the underlining API call? |
It does. To eliminate two api calls when doing one operation like subscribing to a topic, we would have to keep a pubsub.subscribe({
topic: 'name',
name: 'sub-name'
}, function(err) {}); My proposals have mainly been for creating a clear hierarchy: Topic > Subscription. However, situations like above make that tricky to pull off efficiently. |
So do you think that hierarchy matters over performance? |
Sometimes. In this case, I'm not sure, but am leaning towards sticking to a similar API to what we have on master, just with a fresh coat of paint. What are your thoughts? (and anyone else here?) |
How about to just allow creating Topic/Subscription object without any API calls? |
🤖 I have created a release *beep* *boop* --- ## [1.4.0](googleapis/nodejs-service-control@v1.3.0...v1.4.0) (2022-03-23) ### Features * promote to stable ([#122](googleapis/nodejs-service-control#122)) ([d4a1030](googleapis/nodejs-service-control@d4a1030)), closes [#8](googleapis/nodejs-service-control#8) [#98](googleapis/nodejs-service-control#98) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
* feat: support regapic LRO Use gapic-generator-typescript v2.15.1. PiperOrigin-RevId: 456946341 Source-Link: googleapis/googleapis@88fd18d Source-Link: googleapis/googleapis-gen@accfa37 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYWNjZmEzNzFmNjY3NDM5MzEzMzM1YzY0MDQyYjA2M2MxYzUzMTAyZSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [2.1.0](googleapis/nodejs-service-usage@v2.0.0...v2.1.0) (2022-06-30) ### Features * support regapic LRO ([#98](googleapis/nodejs-service-usage#98)) ([b433238](googleapis/nodejs-service-usage@b433238)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
* chore(main): release 2.1.0 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
* feat: Add Secure Boot support to TPU v2alpha1 API PiperOrigin-RevId: 474644226 Source-Link: googleapis/googleapis@f90b329 Source-Link: googleapis/googleapis-gen@4ad8763 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNGFkODc2M2JkZTY3NmY5MmEzZWI3MDc1M2FlMWNmZWQwZTgxMzg3ZSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [2.2.0](https://github.com/googleapis/nodejs-cloud-tpu/compare/v2.1.0...v2.2.0) (2022-11-10) ### Features * Add Secure Boot support to TPU v2alpha1 API ([#98](https://github.com/googleapis/nodejs-cloud-tpu/issues/98)) ([e4fc278](https://github.com/googleapis/nodejs-cloud-tpu/commit/e4fc27883278b8161bb7ad598dd83021e2467d99)) ### Bug Fixes * Allow passing gax instance to client constructor ([#96](https://github.com/googleapis/nodejs-cloud-tpu/issues/96)) ([d636ecf](https://github.com/googleapis/nodejs-cloud-tpu/commit/d636ecf4798258a71f289bd6a6add2cf45e6a2cb)) * Better support for fallback mode ([#91](https://github.com/googleapis/nodejs-cloud-tpu/issues/91)) ([a291abd](https://github.com/googleapis/nodejs-cloud-tpu/commit/a291abd4a0418eb375f9c4a27f19735afee4acca)) * Change import long to require ([#92](https://github.com/googleapis/nodejs-cloud-tpu/issues/92)) ([5de09bb](https://github.com/googleapis/nodejs-cloud-tpu/commit/5de09bb8786a790ff5a6d643f8493b6f6ea3c4ec)) * **deps:** Use google-gax v3.5.2 ([#104](https://github.com/googleapis/nodejs-cloud-tpu/issues/104)) ([86b8617](https://github.com/googleapis/nodejs-cloud-tpu/commit/86b86173fe7f8dd33e5cb6abb683f32d148670c6)) * Do not import the whole google-gax from proto JS ([#1553](https://github.com/googleapis/nodejs-cloud-tpu/issues/1553)) ([#95](https://github.com/googleapis/nodejs-cloud-tpu/issues/95)) ([e4289c1](https://github.com/googleapis/nodejs-cloud-tpu/commit/e4289c164ea2123947328ceddfb09cf083e50a19)) * Preserve default values in x-goog-request-params header ([#97](https://github.com/googleapis/nodejs-cloud-tpu/issues/97)) ([42310b7](https://github.com/googleapis/nodejs-cloud-tpu/commit/42310b7a99b51320e9924f8ecd1d75513b28b598)) * Regenerated protos JS and TS definitions ([#107](https://github.com/googleapis/nodejs-cloud-tpu/issues/107)) ([7e3cba5](https://github.com/googleapis/nodejs-cloud-tpu/commit/7e3cba5fd489373a7c460369d37f04c32e3cd9c3)) * Remove pip install statements ([#1546](https://github.com/googleapis/nodejs-cloud-tpu/issues/1546)) ([#94](https://github.com/googleapis/nodejs-cloud-tpu/issues/94)) ([7a26fe6](https://github.com/googleapis/nodejs-cloud-tpu/commit/7a26fe63b51c661bdd22bfafecc3d4291247dddf)) * use google-gax v3.3.0 ([e4289c1](https://github.com/googleapis/nodejs-cloud-tpu/commit/e4289c164ea2123947328ceddfb09cf083e50a19)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Source-Link: googleapis/synthtool@d229a12 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-nodejs:latest@sha256:74ab2b3c71ef27e6d8b69b1d0a0c9d31447777b79ac3cd4be82c265b45f37e5e
* feat: support regapic LRO Use gapic-generator-typescript v2.15.1. PiperOrigin-RevId: 456946341 Source-Link: googleapis/googleapis@88fd18d Source-Link: googleapis/googleapis-gen@accfa37 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYWNjZmEzNzFmNjY3NDM5MzEzMzM1YzY0MDQyYjA2M2MxYzUzMTAyZSJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [2.0.1](googleapis/nodejs-essential-contacts@v2.0.0...v2.0.1) (2022-06-30) ### Bug Fixes * **docs:** describe fallback rest option ([#98](googleapis/nodejs-essential-contacts#98)) ([1ed45c6](googleapis/nodejs-essential-contacts@1ed45c6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
... chore: update gapic-generator-ruby to the latest commit chore: release gapic-generator-typescript 1.5.0 Committer: @miraleung PiperOrigin-RevId: 380641501 Source-Link: googleapis/googleapis@076f7e9 Source-Link: googleapis/googleapis-gen@27e4c88
🤖 I have created a release \*beep\* \*boop\* --- ### [1.3.3](https://www.github.com/googleapis/nodejs-bigquery-connection/compare/v1.3.2...v1.3.3) (2021-06-22) ### Bug Fixes * make request optional in all cases ([#98](https://www.github.com/googleapis/nodejs-bigquery-connection/issues/98)) ([779e30b](https://www.github.com/googleapis/nodejs-bigquery-connection/commit/779e30b0fbd9b2c1ed7b826c41d17e679d3d60db)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
* chore(deps): update dependency uuid to v3.3.0 * Update package.json
* chore(deps): update dependency uuid to v3.3.0 * Update package.json
9/30/14:
PR: #245
9/29/14:
http://stephenplusplus.github.io/gcloud-node/#/docs/master/pubsub
9/22/14:
Outdated:
Initializing
Topics
Subscriptions
Example Usage
Create a topic and publish a message
Publish a message to a topic
Create a subscription on a topic to listen for messages
Get an existing subscription on a topic to listen for messages, and auto ack them.
Old recommended api, for historic purposes:
The text was updated successfully, but these errors were encountered: