-
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
Automatically create topic / subscription if not found. #445
Comments
Do you mean a Topic and Subscription object should create itself if necessary after you attempt to use it? I'm ok with this idea, but what's the trouble with manually creating the topic/sub before using it? |
Yes. The trouble with manually creating it before use leads to having to write unnecessary code to handle creating it if necessary- unless you meant creating it with a bootstrap script beforehand outside of the application context. In that case, it's just inconvenient for no real benefit. I could see myself forgetting to create new subscriptions/topics while iterating new features. |
The approach we have taken for every object in this library is that you can create one or reference an existing one. If we changed this, it would need to be changed everywhere and would require two API requests rather than one, causing increased delays for responses. Now actually, the latest pubsub v1beta2 (in this library master branch) call to create a topic/subscription is a PUT call meaning its idempotent (basically no matter how many times you call it, it won't cause more effects than if you call it just once). This means that you can effectively "create or get" by always creating the topic regardless of whether it exists or not. If we know we don't need to create it, it would be silly to try to every time we want to do something with it, this would multiply every request time by a factor of two and provide less stability overall. Hope this makes sense. Let me know if there's a smart compromise that could be made here. |
Hmm. I definitely don't want to make a recommendation that would go against the grain of the library as a whole. Instead of trying to create every time, couldn't the "use existing" method create if the response is a 404? That should only add 2 additional RPCs (create, then re-submit whatever request originally gave the 404) and only once. Similar to the var topic = pubsub.topic('my-topic', {autoCreate: true});
topic.publish(message); // will catch 404, create topic, and then publish the message.
var topic2 = pubsub.topic('my-topic-2');
topic2.publish(message); // will behave as today. |
I like this idea. |
There might be some useful information to abstract from this pr when I had basically this exact behavior! #107 |
Interesting conversation there, and @rakyll seems to be pretty hardline on avoiding "magic". Two thoughts:
|
Here's another example to further illustrate the need for this. I followed the "encouraged" flow by writing a one-off script to create pub/sub topics and subscriptions ahead of time: if (!module.parent) {
console.log(
"Creating pub/sub topics and subscriptions on project %s.",
config.gcloud.projectId);
pubsub.createTopic(topicName, function(err, topic){
if(err && err.code == 409 && err.errors && err.errors[0].reason == "alreadyExists") {
console.log("Topic %s already exists.", topicName);
topic = pubsub.topic(topicName);
} else if(err || !topic) {
throw err;
} else {
console.log('Created topic %s', topicName);
}
topic.subscribe(subscriptionName, function(err, subscription){
if(err && err.code == 409 && err.errors && err.errors[0].reason == "alreadyExists") {
console.log("Subscription %s already exists.", subscriptionName);
} else if(err) {
throw err;
} else{
console.log('Created subscription %s', subscriptionName);
}
console.log('Done');
process.exit();
});
});
} Definitely seems a bit unwieldy for just one topic & subscription. Of course, if there were a |
I have a little concern. With the current version of Cloud Pub/Sub, a topic without any subscriptions will throw away your messages until the first subscription will be created and attached to it. So if we auto create on publish, then the messages subsequently published to this topic will go /dev/null, the situation will continue until you create the first subscription. |
Also, hopefully we will start providing gcloud pubsub subcommand, as well as Developer Console UI, so the first time burden will be mitigated. |
I personally think it's a better to document that a topic with no subscriptions doesn't deliver messages and add the autoCreate option than to keep the developer experience we have now. The experience we have now is quite painful and frustrating. Do we have any idea when |
We welcome contributions too so feel free to mock something up for this. I'm a fan of the autoCreate flag to reduce magic. We have no currently planned release date for the next version unfortunately. We're awaiting library reviews from various teams at Google. I hope to have an update for you soon! |
Not so far away. For a time being, you may find it useful to use my command line tool: Although it still doesn't support push subscription, it can be used for other operations (create, list, publish, pull, etc). |
@tmatsuo this is helpful, but for the context of incorporating pub/sub into the end-to-end node on cloud platform sample app, it would be strange to ask the developers to use a python tool. I'll try to mock something up this week. This actually shouldn't be too complex to incorporate. |
Got it, however, I still don't recommend that you spend a lot of time implementing it. It might become more complex than you think, if you want it to be perfect. For example, what happens if creation fails? Also, in the worst case, what happens if the creation fails, but the subsequent API calls return 404 error (it's extremely unlikely, but it can still happen e.g. bad config push etc). So instead, how about to use the API explorer for a time being? |
I meant to say, if the creation suceeds, but..., in the last example. |
I think we've covered this in #465, which was merged to master. |
I would like to share code to solve this problem.
After run it by
you can go to pages http://localhost:8085/v1/projects/pure-plaform/topics and see |
Another solution with curl :
and the full version :
|
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/792bebba-dea1-4f73-8394-fb548c6dd86b/targets - [ ] To automatically regenerate this PR, check this box.
🤖 I have created a release \*beep\* \*boop\* --- ## [3.11.0](https://www.github.com/googleapis/nodejs-asset/compare/v3.10.0...v3.11.0) (2021-01-09) ### Features * adds style enumeration ([#445](https://www.github.com/googleapis/nodejs-asset/issues/445)) ([28c3612](https://www.github.com/googleapis/nodejs-asset/commit/28c361225ab0cdc2d4b141b5ef02cac5f257a85b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/8d5e906d-0de4-4e28-b374-7d5fd4a1ce62/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@1c92077
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/8d5e906d-0de4-4e28-b374-7d5fd4a1ce62/targets - [ ] To automatically regenerate this PR, check this box. Source-Link: googleapis/synthtool@1c92077
Regenerated the library using [gapic-generator-typescript](https://github.com/googleapis/gapic-generator-typescript) v1.2.1.
🤖 I have created a release \*beep\* \*boop\* --- ### [2.2.1](https://www.github.com/googleapis/nodejs-dataproc/compare/v2.2.0...v2.2.1) (2021-01-09) ### Bug Fixes * **browser:** check for fetch on window ([64fd55c](https://www.github.com/googleapis/nodejs-dataproc/commit/64fd55c3d42281f01dbabab5d823039dc5a26416)) * do not modify options object, use defaultScopes ([#445](https://www.github.com/googleapis/nodejs-dataproc/issues/445)) ([c89bc66](https://www.github.com/googleapis/nodejs-dataproc/commit/c89bc662bd8387c82581a02026be5a6d6a40f5af)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@types/mocha](https://github.com/DefinitelyTyped/DefinitelyTyped) | [`^8.0.0` -> `^9.0.0`](https://renovatebot.com/diffs/npm/@types%2fmocha/8.2.3/9.1.1) | [![age](https://badges.renovateapi.com/packages/npm/@types%2fmocha/9.1.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@types%2fmocha/9.1.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@types%2fmocha/9.1.1/compatibility-slim/8.2.3)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@types%2fmocha/9.1.1/confidence-slim/8.2.3)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: "after 9am and before 3pm" (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-scheduler).
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node) ([source](https://github.com/DefinitelyTyped/DefinitelyTyped)) | [`^16.0.0` -> `^18.0.0`](https://renovatebot.com/diffs/npm/@types%2fnode/16.18.3/18.11.9) | [![age](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/compatibility-slim/16.18.3)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/confidence-slim/16.18.3)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-containeranalysis). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yNDEuMTEiLCJ1cGRhdGVkSW5WZXIiOiIzNC4xMS4xIn0=-->
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/792bebba-dea1-4f73-8394-fb548c6dd86b/targets - [ ] To automatically regenerate this PR, check this box.
🤖 I have created a release \*beep\* \*boop\* --- ## [3.11.0](https://www.github.com/googleapis/nodejs-asset/compare/v3.10.0...v3.11.0) (2021-01-09) ### Features * adds style enumeration ([#445](https://www.github.com/googleapis/nodejs-asset/issues/445)) ([28c3612](https://www.github.com/googleapis/nodejs-asset/commit/28c361225ab0cdc2d4b141b5ef02cac5f257a85b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/node) ([source](https://github.com/DefinitelyTyped/DefinitelyTyped)) | [`^16.0.0` -> `^18.0.0`](https://renovatebot.com/diffs/npm/@types%2fnode/16.18.3/18.11.9) | [![age](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/compatibility-slim/16.18.3)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@types%2fnode/18.11.9/confidence-slim/16.18.3)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-talent). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4yNDEuMTEiLCJ1cGRhdGVkSW5WZXIiOiIzNC45LjIifQ==-->
This PR was generated using Autosynth. 🌈 Synth log will be available here: https://source.cloud.google.com/results/invocations/addf93bb-0c57-4d1f-8f4e-08eaba7ddb1c/targets - [ ] To automatically regenerate this PR, check this box. (May take up to 24 hours.) Source-Link: googleapis/synthtool@c6706ee Source-Link: googleapis/synthtool@b33b0e2 Source-Link: googleapis/synthtool@898b38a
Presently I have some code to publish to a particular topic, creating it if necessary:
And similar code to listen for messages to a topic. It goes the other way around - trying to create one at first then re-using if it's already there.
According to this comment, it seems that the intended usage is to create the topic and subscription outside of the application content and never touch them afterwards. It's not clear in the documentation that this is expected.
I think it would be great to either:
I would personally prefer the former as it removes the need to write a "bootstrapping" script and simplifies the user-facing API.
The text was updated successfully, but these errors were encountered: