Skip to content
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

Reactor HTTP Endpoint changes breaks docs + tutorials #765

Closed
mattheworiordan opened this issue Nov 11, 2019 · 7 comments
Closed

Reactor HTTP Endpoint changes breaks docs + tutorials #765

mattheworiordan opened this issue Nov 11, 2019 · 7 comments
Assignees
Labels
api-reference Anything that appears below the API reference blue fold bug A broken link, image etc

Comments

@mattheworiordan
Copy link
Member

mattheworiordan commented Nov 11, 2019

The new Reactor Integration work for IFTT, Zapier and Cloudflare has gone live. As part of this work, we also consolidated Webhooks & HTTP Custom Endpoints into just "HTTP Endpoint".

We have a number of issues now:

  • The help copy under "Single request / Batch request" is incorrect. It states "Batch request will send events triggered concurrently in the same request" which is not true. It batches requests based on a time period of 1 second, and with a max number of messages per envelope. The docs at [https://www.ably.io/documentation/general/webhooks|https://www.ably.io/documentation/general/webhooks] describe how this works.
  • The "Enveloped" checkbox I believe does not apply in Batch mode (old style webhooks). I think the Envelope is required given they are batched. @SimonWoolf can you confirm?
  • I am not sure it makes sense to put "Request Mode" under headers, when enveloped & encoding options sit below the source info. I think how the data is delivered should be grouped together.
  • The documentation for Webhooks is now all wrong - see [https://www.ably.io/documentation/general/webhooks|https://www.ably.io/documentation/general/webhooks]
  • The tutorials for Webhooks etc. should be reviewed. as I suspect they are also now partly wrong.
  • The help copy about how Envelopes works also needs to be revisited now
  • I question whether we should call this HTTP Endpoint in Reactor and just call this Webhooks instead. Customers are far more familiar with that concept, and our docs all refer to Webhooks. Review the Reactor page too www.ably.io/reactor.
  • The links inside the modal to find out more, or get help on a field, should open new tabs. That is when opening a new tab makes sense because the user is in the middle of a form set up process.
  • I think Webhooks should actually come first in the New REactor window because then you have 1 Webhook, two API Integration platforms IFTT & Zapier, and the rest that follow are all functions.
  • The text "The URL of your server we will POST a summary of events to in JSON format" is wrong given you can choose a different encoding (MsgPack).
  • Scrolling is broken in the new reactor event modal when it is higher than the window height, you cannot scroll down to the bottom of the modal.
  • When using in API Consumer, I think we should make it clear that batch request mode is not supported, as opposed to just hiding that option.

Whilst looking at this, we may additional want to consider:

┆Issue is synchronized with this Jira Bug by Unito

@mattheworiordan mattheworiordan added bug A broken link, image etc api-reference Anything that appears below the API reference blue fold labels Nov 11, 2019
@SimonWoolf
Copy link
Member

The "Enveloped" checkbox I believe does not apply in Batch mode (old style webhooks). I think the Envelope is required given they are batched. @SimonWoolf can you confirm?

Yeah - traditional batched webhooks have their own custom structure https://www.ably.io/documentation/general/webhooks#example-channel-message different from all the other rules for historical reasons, and that structure doesn't take an enveloped option

@ably-sync-bot
Copy link

➤ Tony Bedford commented:

(All) Tutorials will be addressed (eventually) as part of https://ably.atlassian.net/browse/DOC-593 ( https://ably.atlassian.net/browse/DOC-593|smart-link )

@ably-sync-bot
Copy link

➤ Tony Bedford commented:

{quote}When using in API Consumer, I think we should make it clear that batch request mode is not supported, as opposed to just hiding that option.{quote}

Not sure what “API Consumer“ means, but both batch and single are now supported for all rule types.

@ably-sync-bot
Copy link

➤ Tony Bedford commented:

I think webhooks docs (https://ably.com/documentation/general/events ( https://ably.com/documentation/general/events|smart-link )) have been updated since this was raised.

@ably-sync-bot
Copy link

➤ Tony Bedford commented:

{quote}I am not sure it makes sense to put "Request Mode" under headers, when enveloped & encoding options sit below the source info. I think how the data is delivered should be grouped together.{quote}

I’m not sure this is a docs issue. If there’s still a concern I can raise it with the Product team.

@ably-sync-bot
Copy link

➤ Tony Bedford commented:

{quote}The help copy about how Envelopes works also needs to be revisited now{quote}

Text now says:

Messages delivered through Reactor are wrapped in an Ably envelope by default that contains metadata about the message and its payload. The form of the envelope depends on whether it is part of a Webhook/Function ( https://ably.com/general/events ) or a Queue/Firehose ( https://ably.com/documentation/general/queues#enveloped ) rule. For everything besides Webhooks, you can ensure you only get the raw payload by unchecking "Enveloped" when setting up the rule.

We should not be using the term “Reactor“ (we have another ticket to address this), but I think otherwise the text is now OK isn’t it?

@ably-sync-bot
Copy link

➤ Tony Bedford commented:

Closing, as I think all the items in this ticket have been addressed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-reference Anything that appears below the API reference blue fold bug A broken link, image etc
Development

No branches or pull requests

4 participants