Conversation
praecipula
left a comment
praecipula
left a comment
There was a problem hiding this comment.
All in all it looks pretty good! Can you double check the nits / little points of clarity and consider incorporating them in?
Thanks!
| create_parser.add_argument( | ||
| "--target", | ||
| type=str, | ||
| help="The webhook URL to receive the HTTP POST", |
There was a problem hiding this comment.
Can we describe an example of this if they run as per the instructions, just to make it clear that they're specifying a callback address they're setting up with the server file? I believe that'd be something like http://[computer_address]:3000/receive_webhook (if we change the route below...)
| ``` | ||
| ASANA_ACCESS_TOKEN=<your Asana PAT> SLACK_TOKEN=<your Slack API token> ./server.py | ||
| ``` | ||
|
|
There was a problem hiding this comment.
It might be worth pointing out that the computer that this is running on needs to be DNS-addressable or have a public IP address that Asana can get to. This is often a point of confusion / friction for developers.
| load_webhook_secret(), msg=req.data, digestmod=sha256 | ||
| ).hexdigest() | ||
|
|
||
| return hmac.compare_digest(computed_signature, request_signature) |
There was a problem hiding this comment.
👏 since this is optional to get a webhook payload I applaud you including it (I have a feeling a lot of apps don't even check...)
|
|
||
|
|
||
| @app.route("/slack_webhook", methods=["POST"]) | ||
| def slack_webhook(): |
There was a problem hiding this comment.
Nit: I think it might be clearer to name this something a bit more clear that this is the callback url for Asana and not for, say, one from Slack. How do you feel about recieve_webhook or receive_asana_webhook?
3 participants
I'm going to move your branch to a PR for easier reviewing...