-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add example Apps in additional languages - Python #352
Changes from 4 commits
3b75b06
0be28a1
d73cd8a
28e2b62
3732dd0
8a54962
6a7efeb
d2bc7b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,7 @@ | ||||||||||||||
### Pre-requisite | ||||||||||||||
- have python installed , preferably `>=3.0` | ||||||||||||||
- install the requirements mentioned in the `requirement.txt` | ||||||||||||||
- make required changes in the `settings.json` to run the app on custom url & port | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||||||||||
|
||||||||||||||
#### RUN the app | ||||||||||||||
* `python3 hello-world.py` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
requests | ||
flask |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"port": "8080", | ||
"root_url": "https://6fe6-103-157-160-168.in.ngrok.io", | ||
"host": "0.0.0.0" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's confusing how there are 3 sources of configuration (hardcode, settings file, and env vars). I'm thinking we should get rid of this file, as it's not something established among the examples already |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,212 @@ | ||
import logging | ||
import os | ||
import json | ||
|
||
import requests | ||
from flask import Flask, request | ||
|
||
logging.basicConfig(level=logging.DEBUG) | ||
|
||
app = Flask(__name__, static_url_path='/static', static_folder='./static') | ||
|
||
DEFAULT_APP_PORT = 8080 | ||
DEFAULT_APP_HOST = 'localhost' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For ease of configurability, maybe we make this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
DEFAULT_ROOT_URL = 'http://localhost:8080' | ||
FORM = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure why this is capitalized. Even though it's only defined once, it's not really a "constant" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replaced with lower case variable names |
||
'title': 'I am a form!', | ||
'icon': 'icon.png', | ||
'fields': [ | ||
{ | ||
'type': 'text', | ||
'name': 'message', | ||
'label': 'message', | ||
'position': 1, | ||
} | ||
], | ||
'submit': { | ||
'path': '/submit', | ||
}, | ||
} | ||
|
||
|
||
@app.route('/manifest.json') | ||
def manifest() -> dict: | ||
return { | ||
'app_id': 'hello-world', | ||
'display_name': 'Hello world app', | ||
'homepage_url': 'https://github.com/mattermost/mattermost-plugin-apps/tree/master/examples/python/hello-world', | ||
'app_type': 'http', | ||
'icon': 'icon.png', | ||
'requested_permissions': ['act_as_bot'], | ||
'on_install': { | ||
'path': '/install', | ||
'expand': { | ||
'app': 'all', | ||
}, | ||
}, | ||
'bindings': { | ||
'path': '/bindings', | ||
}, | ||
'requested_locations': [ | ||
'/channel_header', | ||
'/command' | ||
], | ||
'root_url': os.environ.get('root_url', DEFAULT_ROOT_URL), | ||
} | ||
|
||
|
||
@app.route('/submit', methods=['POST']) | ||
def on_form_submit(): | ||
print(request.json) | ||
return {'type': 'ok', 'text': f'Hello form got submitted. Form data: {request.json["values"]}'} | ||
|
||
|
||
@app.route('/bindings', methods=['GET', 'POST']) | ||
def on_bindings() -> dict: | ||
print(f'bindings called with {request.data}') | ||
return { | ||
'type': 'ok', | ||
'data': [ | ||
{ | ||
# binding for a command | ||
'location': '/command', | ||
'bindings': [ | ||
{ | ||
'description': 'test command', | ||
'hint': '[This is testing command]', | ||
# this will be the command displayed to user as /first-command | ||
'label': 'first-command', | ||
'icon': 'icon.png', | ||
'submit': { | ||
'path': '/first_command', | ||
# expand block is optional. This is more of metadata like which channel, team this command | ||
# was called from | ||
'expand': { | ||
'app': 'all', | ||
# if you want to expand team & channel, ensure that bot is added to the team & channel | ||
# else command will fail to expand the context | ||
# 'team': 'all', | ||
# 'channel': 'all', | ||
}, | ||
Comment on lines
+84
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The bot must be a member of the team and channel, otherwise the expand fails and the request is never sent to the App? It seems that we shouldn't show the command to the user in this case. cc @levb There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am able to get the Steps to reproduce
Sharing raw response from {
"path": "/first_command",
"expand": {
"app": "all"
},
"context": {
"app_id": "hello-world",
"location": "/command/first-command",
"user_agent": "webapp",
"track_as_submit": true,
"mattermost_site_url": "http://localhost:8066",
"developer_mode": true,
"app_path": "/plugins/com.mattermost.apps/apps/hello-world",
"bot_user_id": "3xxj6snbwby19npr1tca3ys9zo",
"bot_access_token": "3mf1popcwjnzxyeps9kqsqeuoc",
"app": {
"app_id": "hello-world",
"webhook_secret": "zrp5e37iejgtixpfqtjt43ibua",
"bot_user_id": "3xxj6snbwby19npr1tca3ys9zo",
"bot_username": "hello-world",
"remote_oauth2": {}
},
"acting_user": {
"id": "kredsjwqwbrp9r41c86b69akno",
"delete_at": 0,
"username": "",
"auth_service": "",
"email": "",
"nickname": "",
"first_name": "",
"last_name": "",
"position": "",
"roles": "",
"locale": "",
"timezone": null,
"disable_welcome_email": false
},
"oauth2": {}
},
"raw_command": "/first-command "
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay so when you say "command will fail to expand the context", it's sort of a silent error, and the app still receives the request. It's just not the full request with all values. I think this is due to new changes to expand (optional vs required), depending on which version of the App framework you're working with. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
}, | ||
} | ||
], | ||
}, | ||
{ | ||
# binding for a command, command with embedded form | ||
'location': '/command', | ||
'bindings': [ | ||
{ | ||
'description': 'test command', | ||
'hint': '[This is testing command]', | ||
# this will be the command displayed to user as /second-command | ||
'label': 'second-command', | ||
'icon': 'icon.png', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should instead be in the same array that begins on line 73, rather than creating the new object on line 95 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed. |
||
'bindings': [ | ||
{ | ||
'location': 'send', | ||
'label': 'send', | ||
'form': FORM | ||
}, | ||
], | ||
} | ||
], | ||
}, | ||
{ | ||
'location': '/channel_header', | ||
'bindings': [ | ||
{ | ||
'location': 'send-button', | ||
'icon': 'icon.png', | ||
'label': 'send hello message', | ||
'form': FORM, | ||
}, | ||
], | ||
}, | ||
], | ||
} | ||
|
||
|
||
@app.route('/ping', methods=['POST']) | ||
def on_ping() -> dict: | ||
logging.debug('ping...') | ||
return {'type': 'ok'} | ||
|
||
|
||
@app.route('/install', methods=['GET', 'POST']) | ||
def on_install() -> dict: | ||
print(f'on_install called with payload , {request.args}, {request.data}', flush=True) | ||
_subscribe_team_join(request.json['context']) | ||
return {'type': 'ok', 'data': []} | ||
|
||
|
||
@app.route('/first_command', methods=['POST']) | ||
def on_first_command(): | ||
print(f'/first_command called ') | ||
response_message = 'Hello! response from /first_command' | ||
return {'type': 'ok', 'text': response_message} | ||
|
||
|
||
@app.route('/bot_joined_team', methods=['GET', 'POST']) | ||
def on_bot_joined_team() -> dict: | ||
context = request.json['context'] | ||
logging.info( | ||
f'bot_joined_team event received for site:{context["mattermost_site_url"]}, ' | ||
f'team:{context["team"]["id"]} name:{context["team"]["name"]} ' | ||
f'{request.args} {request.data}' | ||
) | ||
# Here one can subscribe to channel_joined/left events as these required team_id now to be subscribed, | ||
# hence use the team_id received in the event and make a call for subscribing to channel_joined/left events. | ||
Comment on lines
+154
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The framework should make sure these subscriptions are removed when the bot is removed from this team cc @levb for discussion |
||
# Also supply {'team_id': team_id} in the request body of the subscription | ||
# { | ||
# 'subject': 'bot_joined_team', | ||
# 'call': { | ||
# 'path': '/bot_joined_team', | ||
# 'expand': { | ||
# 'app': 'all', | ||
# 'team': 'all' | ||
# } | ||
# }, | ||
# 'team_id': 'team_id' # get this team_id when bot_joined_team event occurs | ||
# } | ||
return {'type': 'ok', 'data': []} | ||
|
||
|
||
# Subscribing to events. For example, Subscribe to 'bot_joined_team' event | ||
def _subscribe_team_join(context: dict) -> None: | ||
site_url = context['mattermost_site_url'] | ||
bot_access_token = context['bot_access_token'] | ||
url = site_url + 'plugins/com.mattermost.apps/api/v1/subscribe' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
logging.info(f'Subscribing to team_join for {site_url}') | ||
headers = {'Authorization': f'BEARER {bot_access_token}'} | ||
body = { | ||
'subject': 'bot_joined_team', | ||
'call': { | ||
'path': '/bot_joined_team', | ||
'expand': { | ||
'app': 'all', | ||
'team': 'all' | ||
} | ||
}, | ||
} | ||
res = requests.post(url, headers=headers, json=body) | ||
if res.status_code != 200: | ||
logging.error(f'Could not subscribe to team_join event for {site_url}') | ||
else: | ||
logging.debug(f'subscribed to team_join event for {site_url}') | ||
|
||
|
||
if __name__ == '__main__': | ||
# load settings | ||
root_dir = os.path.dirname(os.path.dirname(__file__)) | ||
with open(f'{root_dir}/settings.json') as config_file: | ||
settings = json.load(config_file) | ||
for env_name, env_value in settings.items(): | ||
os.environ[env_name] = env_value | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The environment variables should take precedence over the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
|
||
app.run( | ||
debug=True, | ||
host=os.environ.get('host', DEFAULT_APP_HOST), | ||
port=int(os.environ.get('port', DEFAULT_APP_PORT)), | ||
use_reloader=False, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pip3 install -r requirements.txt
right? We should explain that hereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.