display response - gpub

[jigyasak05]

Signed-off-by: Jigyasa jigyasakhaneja05@gmail.com

Pull request

Description

Display the response received from GCP when a subscription is created.

Related

#724

Checklist

• The RFC, if required, has been submitted and approved
• Any user-facing impact of the changes is reflected in docs.tremor.rs
• The code is tested
• Use of unsafe code is reasoned about in a comment
• Update CHANGELOG.md appropriately, recording any changes, bug fixes, or other observable changes in behavior
• The performance impact of the change is measured (see below)

Performance

[jigyasak05]

The commented fields were not included in the response because the data type was not compatible.

[coveralls]

Coverage increased (+0.01%) to 83.972% when pulling 3098315 on jigyasak05:pubsub-connector into 09e67b9 on tremor-rs:main.

[mfelsche]

Looking good, just one single question about the response format :)

[mfelsche]

It seems that the response that is sent back below will always be an array of records, even if the event is not batched (has only one value). Is that intended? The docs PR tremor-rs/tremor-www-docs#160 describes it as an object. Maybe below you check if the response vec has 1 element and the event is not batched, then you can extract the first element and use this as response Value, otherwise use the full vec.

[jigyasak05]

Yes true, In case of gpub (sink), since we can only send one message at a time currently - we'll always have one value in the response🤔 So I just extract the first element instead of sending the vec as response, right?

[darach]

This could just be something we can clarify in the docs? Batching in tremor is very different from how the pubsub APIs work and the responses from G pubsub are a vector of messages - instead of object perhaps we should have stated value instead - and explained. a little how the underlying messages are mapped to values in tremor?

[Licenser]

I think the confusion stems from that this seems to be returning an array with a struct

[{
  "subscription": "projects/<project-id>/subscriptions/<subscription-name>",
  "topic": "projects/<project-id>/topics/<topic-name>",
  "ack_deadline_seconds": `<ack_deadline_seconds>`,
  "retain_acked_messages": `<retain_acked_messages>`,
  "enable_message_ordering": `<enable_message_ordering>`,
  "message_retention_duration": `<message_retention_duration>`,
  "filter": "<filter>",
  "detached": `<detached>`
}]

while the documentation states it returns a struct:

{
  "subscription": "projects/<project-id>/subscriptions/<subscription-name>",
  "topic": "projects/<project-id>/topics/<topic-name>",
  "ack_deadline_seconds": `<ack_deadline_seconds>`,
  "retain_acked_messages": `<retain_acked_messages>`,
  "enable_message_ordering": `<enable_message_ordering>`,
  "message_retention_duration": `<message_retention_duration>`,
  "filter": "<filter>",
  "detached": `<detached>`
}

I would opt for changing the reply to match the documentation not the documentation to match the reply as at least from the docs there seems to be no reason to return an array with a singular element it would only making consuming the data harder for users.

[jigyasak05]

I actually get this response but I only mentioned the struct part because the GCP response consists of only that:

{
  "data": [
    {
      "subscription_name": "projects/<project-id>/subscriptions/tremor-test",
      "topic": "projects/<project-id>/topics/tremor",
      "ack_deadline_seconds": 10,
      "retain_acked_messages": true,
      "enable_message_ordering": true,
      "message_retention_duration": 604800,
      "filter": "",
      "detached": false
    }
  ],
  "meta": {}
}

[Licenser]

That would be wonderful :)! 🚀

[mfelsche]

To add some context here:

In Tremor it is possible that a single event actually contains more than one. This can be the case when the batch operator was used. In that case the event value contains multiple nested events over which we can iterate with Event::value_iter() (as done above) or Event::value_meta_iter(). This is kind of a special case that makes our life as offramp developers harder :/

So, if the event arriving at the gpub offramp is batched you get multiple entries back from Event::value_iter and you will do multiple calls to the gpub api, one for each value. The response vec will contain multiple values. In that case mark the reply event also as batched and take care that the actual response value is an array of records (which is the case right now) and that it has the same format as as is used in the batch operator: https://github.com/tremor-rs/tremor-runtime/blob/main/tremor-pipeline/src/op/generic/batch.rs#L97-L104

If it is not batched (the incoming event has event.is_batch = false, you could extract the first element from the response vec and use that as the event data, not an array. I think this is the most consistent handling here.

[mfelsche]

Sorry for stirring up the discussion again, i didnt see the above discussion when i was writing my lament. :)

[jigyasak05]

Alright!! I'll fix it ^_^

[jigyasak05]

Updated it

[jigyasak05]

Why does this build fail? This test was passing earlier :-(

[Licenser]

DroneCi is very temperamental, nothing you did wrong, nothing we did wrong either, sadly it's just how drone works. But we have it as optional so it's safe to ignore it :)

[jigyasak05]

Okay cool! I had added the comment ^_^

[Licenser]

🚀 nicely done!