nodejs-pubsub icon indicating copy to clipboard operation
nodejs-pubsub copied to clipboard

Add samples for RetryPolicy and filtering

Open feywind opened this issue 5 years ago • 19 comments

This is in response to these two issues:

https://github.com/googleapis/nodejs-pubsub/issues/974 https://github.com/googleapis/nodejs-pubsub/issues/976

It looks like the support is in for us to do this, but some samples could go a long way toward making it accessible.

feywind avatar Jun 12 '20 17:06 feywind

no any progress?

men232 avatar Aug 14 '20 10:08 men232

@feywind any progress here? Tried to make it work with setMetadata in a number of ways, but keep getting errors.

INVALID_ARGUMENT: Invalid update_mask provided in the UpdateSubscriptionRequest: retry_policy is not a known Subscription field. Note that field paths must be of the form 'push_config' rather than 'pushConfig'.

josedonizetti avatar Aug 19 '20 20:08 josedonizetti

@josedonizetti @sagimonza (from #974) I was able to get this working with the current v2.5.0 release.

Gist demoing it both at sub create time and via setMetadata: https://gist.github.com/mgabeler-lee-6rs/69a9b0e368d061b1935a7aa428e217b6

... I do get the error Jose mentions if I try to run it against the emulator however (have to comment out topic.setMetadata to get to the failure for the subscription however).

Interestingly, the emulator acts as if it supports setting the retryPolicy on create of a subscription, and will even return it back to you in getMetadata after that ... but it doesn't actually implement the retry policy.

mgabeler-lee-6rs avatar Aug 20 '20 05:08 mgabeler-lee-6rs

It is quite frustrating that it seems you can't use almost any of the keys in setMetadata in the emulator. Makes it hard to have code using this library that can get tested in CI or used by developers on localhost :disappointed:

mgabeler-lee-6rs avatar Sep 03 '20 22:09 mgabeler-lee-6rs

does anyone know, how to set a filter for a subscription using this sdk? any docs??

mahendraHegde avatar Sep 16 '20 13:09 mahendraHegde

does anyone know, how to set a filter for a subscription using this sdk? any docs??

  1. Make sure that you use latest version of this lib.
  2. Just pass filter property into subscription creating

For example

filter: [ attributes.platform = "dev", attributes.label = "broker" ].join(' AND '),

men232 avatar Sep 16 '20 17:09 men232

It is quite frustrating that it seems you can't use almost any of the keys in setMetadata in the emulator. Makes it hard to have code using this library that can get tested in CI or used by developers on localhost 😞

It seems the issues with setMetadata are not unique to the emulator. Trying to use it against real pubsub and it yields errors like this:

INVALID_ARGUMENT: Invalid update_mask provided in the UpdateTopicRequest: 'messageStoragePolicy' is not a known Topic field. Note that field paths must be of the form 'push_config' rather than 'pushConfig'.

I've attempted to use different variations, such as snake-casing, to coerce it:

message_storage_policy: {
  allowed_persistence_regions: ['europe-west2'],
},

This avoids the INVALID_ARGUMENT error, however no combination that I've tried seems to take any affect on the actual Topic in GCP.

UPDATE:

So it appears that somewhere along the line, the update mask value of 'messageStoragePolicy' is being converted to 'messageStoragepolicy' - where did that lower-case p come from???

image

when I debug into call-stream.sendMessageWithContext, I can see the serialised message still contains the valid upper-case P:

>>message.toString()
'
6
$projects/{OMITTED}/topics/TEST-62
europe-west2
messageStoragePolicy'

It's not clear to me at what point this is becoming a lower-case p.

fiendfiend avatar Oct 15 '20 18:10 fiendfiend

It is quite frustrating that it seems you can't use almost any of the keys in setMetadata in the emulator. Makes it hard to have code using this library that can get tested in CI or used by developers on localhost disappointed

Is there somewhere we can raise issues on the emulator?

ThomWright avatar Nov 24 '20 15:11 ThomWright

Is there somewhere we can raise issues on the emulator?

On the theory that the emulator is part of the cloud sdk, I reported https://issuetracker.google.com/issues/165694293. It at least received an acknowledgement, though it's 50/50 to me whether that ack was actually from a human, or just a well written bot/template.

mgabeler-lee-6rs avatar Nov 24 '20 15:11 mgabeler-lee-6rs

Taking a look at some old issues in here, a few comments:

  • @mgabeler-lee-6rs To be completely honest, there has been a lot of discussion about who owns the emulators and updates them with new features. I'm not sure there's been a resolution, but that seems like a reasonable place to file the request. Thanks for doing that. I think the intent is basically that it will cover the most commonly used features, but I agree that's a bit of a suboptimal experience.
  • @fiendfiend As far as the lower case p, I don't see that in the library itself, so I'm wondering if there's a case issue somewhere that you're passing settings to the library? If you're still having this issue and could paste the lines of code that call into the library to set the settings, that'd be helpful.
  • The added samples are definitely still in the queue to be done, so that hasn't been forgotten. We've been talking about some ways to better organize ongoing requests and questions, so I'm hoping that will help keep these issues from becoming stale.

feywind avatar Jan 15 '21 21:01 feywind

@feywind Any updates on the samples?

chrisatcloudwalk avatar Feb 17 '21 14:02 chrisatcloudwalk

Looking through some issues, it seems like it would look like this:

function main(
  topicName = 'YOUR_TOPIC_NAME',
  subscriptionName = 'YOUR_SUBSCRIPTION_NAME'
) {
  // [START pubsub_create_pull_subscription]
  /**
   * TODO(developer): Uncomment these variables before running the sample.
   */
  // const topicName = 'YOUR_TOPIC_NAME';
  // const subscriptionName = 'YOUR_SUBSCRIPTION_NAME';

  // Imports the Google Cloud client library
  const {PubSub} = require('@google-cloud/pubsub');

  // Creates a client; cache this for further use
  const pubSubClient = new PubSub();

  async function createSubscription() {
    // Creates a new subscription
    await pubSubClient.topic(topicName).createSubscription(subscriptionName, {
        filter: 'attributes.my-attr = my-filter'
    });
    console.log(`Subscription ${subscriptionName} created.`);
  }

  createSubscription().catch(console.error);
  // [END pubsub_create_pull_subscription]
}

C-Powers avatar Dec 16 '21 00:12 C-Powers

@feywind & @anguillanneuf Just a heads up, the just-merged PR that closed this issue did not fully address the issue. The PR only added samples for subscriptions with filtering—not for subscriptions with a retry policy.

WesCossick avatar Feb 23 '22 20:02 WesCossick

@WesCossick You are right. The sample is going to be published here: https://cloud.google.com/pubsub/docs/filtering#creating_subscriptions_with_filters. It doesn't have anything to do with RetryPolicy. It's unrelated.

anguillanneuf avatar Feb 23 '22 21:02 anguillanneuf

@WesCossick I thought I'd commented here, sorry - at the moment we don't have a tag for the docs for a subscription retry policy sample, but I'm okay leaving this open to track that.

feywind avatar Feb 23 '22 21:02 feywind

@anguillanneuf pointed me at this:

https://cloud.google.com/pubsub/docs/admin#pubsub_update_push_configuration-nodejs

which is also this:

https://github.com/googleapis/nodejs-pubsub/blob/main/samples/modifyPushConfig.js

You should be able to do a similar thing by calling setMetadata with your retry policy instead of modifyPushConfig. I can add a "non-canonical" sample for that if it would be helpful. (And/or try to get a sample tag made, but that would trigger a bunch of other languages needing those samples.)

feywind avatar Feb 23 '22 22:02 feywind

@feywind is right. @WesCossick, are you able to modify that sample to update retry_policy? We don't have plans to add a sample for every update operation. Hope you understand.

anguillanneuf avatar Feb 23 '22 22:02 anguillanneuf

I'd have to dig into the code where we use retry policies and get back on that. I just wanted to quickly point out that this issue was created specifically to combine #974 and #976 into a single issue, and then those two issues were closed, but the PR ultimately only addressed #976.

WesCossick avatar Feb 23 '22 22:02 WesCossick

@feywind (Maybe you already have, but) can we verify if this is true? https://github.com/googleapis/nodejs-pubsub/issues/974#issuecomment-668226017

We don't need to publish a docs sample but we can include some code snippets as part of the library reference. For examples:

  • https://cloud.google.com/java/docs/reference/google-cloud-pubsub/latest/overview
  • https://cloud.google.com/python/docs/reference/pubsub/latest/upgrading
  • https://cloud.google.com/nodejs/docs/reference/pubsub/latest
  • https://cloud.google.com/nodejs/docs/reference/google-auth-library/latest

anguillanneuf avatar Feb 23 '22 23:02 anguillanneuf

This is what I'm using to create a subscription with metadata such as retry policy in NodeJS:

https://gist.github.com/greensopinion/ce5ebc6a8d76d2978634a089ead082b6

greensopinion avatar Nov 05 '22 17:11 greensopinion

@greensopinion Thanks for the gist. I think we're going to add a sample for this after all. There was some debate about whether the sample browser site was for any sample or only for ones in the doc pages.

feywind avatar Mar 07 '23 21:03 feywind