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

Adds Webhook Documentation #205

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

cforbes
Copy link
Contributor

@cforbes cforbes commented Jul 25, 2017

Fixes # .

Changes proposed:

@Skillz4Killz
Copy link
Collaborator

@cforbes Overall this looks amazing. There is one thing that I believe webhooks are going to be a very crucial and important feature. I see it is currently listed at the bottom of the docs under SDK and errors. Can we possibly move this up. In my opinion, the order should be as follow:

Available Endpoints
Coming Soon Endpoints
Errors
SDK

Copy link
Collaborator

@dominicgunn dominicgunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these callbacks going to be signed? If not, is there any good reason why?

Wouldn't want some poor consumer getting smashed with bad data because their callback URL was exposed.

@svperfecta
Copy link
Contributor

@dominicgunn added a ticket for signing these. It was an oversight, and exactly why we posted these ahead of launch. Thank you very much for the feedback!

@hjstn
Copy link

hjstn commented Aug 15, 2017

Would it be possible to create and delete multiple webhooks in one call? It'd make more sense if many players must be checked at once.

@svperfecta
Copy link
Contributor

svperfecta commented Aug 16, 2017 via email

@schneefux
Copy link
Contributor

The number of webhooks that can be created is limited to 100 per app.

We are serving a community tournament with over 200 participants. Is the limitation to 100 hooks fixed? Can you scale it with the token's ratelimit?

"12": "_links",
"13": "_sdks",
"14": "_errors"
"12": "_webhooks"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSON has broken, let's fix it. (Missing a , after "_webhooks").


This endpoint creates a webhook

Note: A verification event will be sent to the url (see supported events above). If the request does not succeed, the webhook will not be created.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we refer to 'supported events', where as previously we referred to 'supported topics'. Should we keep terminology the same?

Copy link
Collaborator

@dominicgunn dominicgunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of things that might need fixing before merging.

@dominicgunn
Copy link
Collaborator

What is the retry logic if a callback fails to reach its destination?

@hjstn
Copy link

hjstn commented Sep 1, 2017

Are there other topics planned for the future? I think topics for when a match starts, or using a spectator name instead of player in tournament servers would be useful.

Copy link
Collaborator

@dominicgunn dominicgunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation seems OK, would like more info on retry procedure.

@svperfecta
Copy link
Contributor

We'll add the retry logic documentation in there. Short summary is that it will retry for several days (i think). We should document the backoff algorithm too. I'll verify this!

@svperfecta
Copy link
Contributor

@justin97530 Match start probably won't happen (but you can log a ticket for it!) Spectators is a great one and we can totally do that.

@PierreAndreis
Copy link
Contributor

Any ETA on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants