Skip to content

WIP: Send Slack request with event data (if handler asks for it)#39

Closed
datashaman wants to merge 3 commits intoslackapi:mainfrom
datashaman:feature/event-request
Closed

WIP: Send Slack request with event data (if handler asks for it)#39
datashaman wants to merge 3 commits intoslackapi:mainfrom
datashaman:feature/event-request

Conversation

@datashaman
Copy link
Contributor

@datashaman datashaman commented Oct 14, 2018 •
edited
Loading

Inspects arity of handler function to ascertain whether it should pass
request with event_data. Does not break current implementations.

Uses inspect module to check arity of the first listener for a specific event type.

If you want an event handler to get the Slack request, define it like this:

@adapter.on('reaction_added')
def reaction_added(event_data, request):
    event = event_data['event']
    if request.headers.get('X-Slack-Retry-Num'):
        print('Event was retried')

NB: A side-effect of this is that all event handlers of the same type must have the same number of arguments!

Resolves #32
Requires (and is based on) #37

@datashaman datashaman force-pushed the feature/event-request branch from 9550f2d to c7ae074 Compare October 14, 2018 10:13
@datashaman
Copy link
Contributor Author

datashaman commented Oct 14, 2018

This could be changed to send just the headers, but I thought that the whole request might make it more useful.

@datashaman
Copy link
Contributor Author

datashaman commented Oct 14, 2018

Please notify me if/when the base PR is merged, and I'll rebase this on new master.

@datashaman datashaman force-pushed the feature/event-request branch from c7ae074 to 8484576 Compare October 14, 2018 10:33
@datashaman datashaman force-pushed the feature/event-request branch from 8484576 to 90dd959 Compare October 14, 2018 10:54
@loot-king
Copy link

loot-king commented Apr 29, 2020

😘

loot-king
loot-king approved these changes Apr 29, 2020
View reviewed changes
Copy link

@loot-king loot-king left a comment

Choose a reason for hiding this comment

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

💇‍♂️

@seratch
Copy link
Contributor

seratch commented Jun 11, 2020

I'm sorry the team took a very long time to respond to this. I'm still not sure if this approach is the best for addressing #32 yet. Allow me to hold off making any decisions about this for now.

@seratch seratch changed the title Send Slack request with event data (if handler asks for it) WIP: Send Slack request with event data (if handler asks for it) Jun 11, 2020
@Goryudyuma Goryudyuma mentioned this pull request Jun 17, 2020
Closed
@seratch seratch changed the base branch from master to main July 8, 2020 09:05
@Lobosque
Copy link

Lobosque commented Sep 12, 2020

@seratch while you decide on what the best approach is, is there any way at all to access the request header while handling an event?

@seratch
Copy link
Contributor

seratch commented Sep 12, 2020

@Lobosque

@seratch while you decide on what the best approach is, is there any way at all to access the request header while handling an event?

As request in Flask is a thread-local value, you can access it just by importing it.

import os
from flask import request
from slackeventsapi import SlackEventAdapter

slack_signing_secret = os.environ["SLACK_SIGNING_SECRET"]
slack_events_adapter = SlackEventAdapter(slack_signing_secret, "/slack/events")

# Example reaction emoji echo
@slack_events_adapter.on("reaction_added")
def reaction_added(event_data):
    print(request.headers)
    event = event_data["event"]

After leaving the above comment, I came to think that directly accessing request is totally fine because this project doesn't have plans to support any other frameworks apart from Flask.

NB: A side-effect of this is that all event handlers of the same type must have the same number of arguments!

Considering this disadvantage, I think we don't have a strong reason to merge this PR's changes.

@seratch seratch mentioned this pull request Sep 12, 2020
Closed
9 tasks
@Conway Conway mentioned this pull request Oct 24, 2020
Merged
@seratch
Copy link
Contributor

seratch commented Feb 11, 2021

As I mentioned in the last comment, you can access Flask's thread-local request object. Let me close this pull request. Thanks for taking the time to send it.

@seratch seratch closed this Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@loot-king loot-king loot-king approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Possible to handle HTTP headers from Slack within event adapter?

4 participants

@datashaman @loot-king @seratch @Lobosque