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

feat: Slack pairings #553

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions app/model/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
import app.model.team as team
import app.model.permissions as permissions
import app.model.project as project
import app.model.pairing as pairing
import app.model.base as base

User = user.User
Team = team.Team
Permissions = permissions.Permissions
Project = project.Project
Pairing = pairing.Pairing
BaseModel = base.RocketModel
112 changes: 112 additions & 0 deletions app/model/pairing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
"""Represent a Pairing between two users."""
from typing import Dict, Any, TypeVar, Type
import uuid
from app.model.base import RocketModel

T = TypeVar('T', bound='Pairing')


class Pairing(RocketModel):
"""Represent a pairing and related fields and methods."""

def __init__(self,
user1_slack_id: str,
user2_slack_id: str):
"""
Initialize the pairing.

The generated ``pairing_id`` property is meant to uniquely
represent a pairing.

:param user1_slack_id: The slack ID of the first user
:param user2_slack_id: The slack ID of the second user
"""
self.pairing_id = str(uuid.uuid4())
self.user1_slack_id = user1_slack_id
self.user2_slack_id = user2_slack_id
Comment on lines +25 to +26
Copy link
Collaborator

Choose a reason for hiding this comment

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

How would 3 people models be represented in the database? Or is it intended that 3 people pairings are a one-off?

self.ttl = "TODO" # TODO

def get_attachment(self) -> Dict[str, Any]:
"""Return slack-formatted attachment (dictionary) for pairing."""
text_pairs = [
('Pairing ID', self.pairing_id),
('User 1 Slack ID', self.user1_slack_id),
('User 2 Slack ID', self.user2_slack_id),
('TTL', self.ttl)
]

fields = [{'title': t, 'value': v if v else 'n/a', 'short': True}
for t, v in text_pairs]
fallback = str('\n'.join(map(str, text_pairs)))

return {'fallback': fallback, 'fields': fields}

@classmethod
def from_dict(cls: Type[T], d: Dict[str, Any]) -> T:
"""
Return a pairing from a dict object.

:param d: the dictionary (usually from DynamoDB)
:return: a Pairing object
"""
p = cls(d['user1_slack_id'], d['user2_slack_id'])
p.pairing_id = d['pairing_id']
p.ttl = d['ttl']
return p

@classmethod
def to_dict(cls: Type[T], p: T) -> Dict[str, Any]:
"""
Return a dict object representing a pairing.

The difference with the in-built ``self.__dict__`` is that this is more
compatible with storing into NoSQL databases like DynamoDB.

:param p: the Pairing object
:return: a dictionary representing a pairing
"""
def place_if_filled(name: str, field: Any):
"""Populate ``udict`` if ``field`` isn't empty."""
if field:
udict[name] = field

udict = {
'pairing_id': p.pairing_id,
'user1_slack_id': p.user1_slack_id,
'user2_slack_id': p.user2_slack_id
}
place_if_filled('ttl', p.ttl)

return udict

@classmethod
def is_valid(cls: Type[T], p: T) -> bool:
"""
Return true if this pairing has no missing fields.

Required fields for database to accept:
- ``__pairing_id``
- ``__user1_slack_id``
- ``__user2_slack_id``

:param pairing: pairing to check
:return: true if this pairing has no missing fields
"""
return len(p.pairing_id) > 0 and\
len(p.user1_slack_id) > 0 and len(p.user2_slack_id) > 0

def __eq__(self, other: object) -> bool:
"""Return true if this pairing is equal to the other pairing."""
return isinstance(other, Pairing) and str(self) == str(other)

def __ne__(self, other: object) -> bool:
"""Return true if this pairing isn't equal to the other pairing."""
return not (self == other)

def __str__(self) -> str:
"""Return all fields of this pairing, JSON format."""
return str(self.__dict__)

def __hash__(self) -> int:
"""Hash the pairing class using a dictionary."""
return self.__str__().__hash__()
10 changes: 9 additions & 1 deletion app/scheduler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
from flask import Flask
from apscheduler.schedulers.background import BackgroundScheduler
from .modules.random_channel import RandomChannelPromoter
from .modules.pairing import PairingSchedule
from .modules.base import ModuleBase
from db.facade import DBFacade
from typing import Tuple, List
from config import Config

Expand All @@ -13,10 +15,12 @@ class Scheduler:

def __init__(self,
scheduler: BackgroundScheduler,
args: Tuple[Flask, Config]):
args: Tuple[Flask, Config],
facade: DBFacade):
"""Initialize scheduler class."""
self.scheduler = scheduler
self.args = args
self.facade = facade
self.modules: List[ModuleBase] = []

self.__init_periodic_tasks()
Expand All @@ -35,3 +39,7 @@ def __add_job(self, module: ModuleBase):
def __init_periodic_tasks(self):
"""Add jobs that fire every interval."""
self.__add_job(RandomChannelPromoter(*self.args))
# Feature toggle the pairings based on the
# existence of the `SLACK_PAIRING_CHANNEL` env variable
if len(self.args[1].slack_pairing_channel) != 0:
self.__add_job(PairingSchedule(*self.args, self.facade))
119 changes: 119 additions & 0 deletions app/scheduler/modules/pairing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
"""Match two Launch Pad member for a private conversation"""
from slack import WebClient
from interface.slack import Bot
from random import shuffle
from .base import ModuleBase
from typing import Dict, List, Any, Set
from flask import Flask
from config import Config
from db.facade import DBFacade
from app.model import Pairing
import logging


class PairingSchedule(ModuleBase):
"""Module that pairs members each ``SLACK_PAIRING_FREQUENCY``"""

NAME = 'Pair members randomly for meetups'

def __init__(self,
flask_app: Flask,
config: Config,
facade: DBFacade):
"""Initialize the object."""
self.bot = Bot(WebClient(config.slack_api_token),
config.slack_notification_channel)
self.channel_id = self.bot.get_channel_id(config.slack_pairing_channel)
self.config = config
self.facade = facade

def get_job_args(self) -> Dict[str, Any]:
"""Get job configuration arguments for apscheduler."""
logging.info(f"Running pairing at cron job: \
{self.config.slack_pairing_frequency}")
cron_frequency = self.config.slack_pairing_frequency.split(' ')
return {'trigger': 'cron',
'minute': cron_frequency[0],
'hour': cron_frequency[1],
'day': cron_frequency[2],
'month': cron_frequency[3],
'day_of_week': cron_frequency[4],
'name': self.NAME}

def do_it(self):
"""Pair users together, and create a private chat for them"""
users = self.bot.get_channel_users(self.channel_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably want to stop execution when we find out that there are no users in the channel (it also removes a check in __pair_users() that checks to see if there are any users in the list).

logging.debug(f"users of the pairing channel are {users}")
matched_user_pairs = self.__pair_users(users)
for pair in matched_user_pairs:
group_name = self.bot.create_private_chat(pair)
logging.info(f"The name of the created group name is {group_name}")
self.bot.send_to_channel("Hello! \
You have been matched by Rocket \
please use this channel to get to know each other!",
Comment on lines +51 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a long string. Can you put it refactor it to be a static member for easy changing?

group_name)
Comment on lines +51 to +54
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should have a configuration option SLACK_PAIRING_TIPS_LINK that can generate a button that links to tips for what to do in a pairing (i.e. "set up video call, talk about x and y") - for Launch Pad this could be a handbook page, for example

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm Can we do this on a follow up or do ya think it's important? Could be a really good first issue for someone!

Copy link
Member

Choose a reason for hiding this comment

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

Yep! Can you add all the TODO items for follow-up issues in the PR description as a checklist, to make sure we don't forget?


def __pair_users(self, users: List[str]) -> List[List[str]]:
"""
Creates pairs of users that haven't been matched before

:param users: A list of slack ids of all users to match

If a pairing cannot be done, then the history of pairings is
purged, and the algorithm is run again.
Comment on lines +58 to +63
Copy link
Collaborator

Choose a reason for hiding this comment

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

Talk about what kind of algorithm is used here please.

"""
# TODO: Clean this up into a more concrete algorithm
logging.info("Running pairing algorithm")
shuffle(users)
already_added = set()
pairs = []
for i, user in enumerate(users):
if user in already_added:
continue
previously_paired = self.__get_previous_pairs(user)
for j in range(i + 1, len(users)):
other_user = users[j]
if other_user not in previously_paired and \
other_user not in already_added:
self.__persist_pairing(user, other_user)
pairs.append([user, other_user])
already_added.add(user)
already_added.add(other_user)
break
not_paired = list(
filter(lambda user: user not in already_added, users))
Comment on lines +83 to +84
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
not_paired = list(
filter(lambda user: user not in already_added, users))
not_paired = [user for user in users if user not in already_added]

Feel free to not use this. I just want everything to fit nicely on one line.

P Y T h O n I C

# If we have an odd number of people that is not 1
# We put the odd person out in one of the groups
# So we might have a group of 3
Comment on lines +85 to +87
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# If we have an odd number of people that is not 1
# We put the odd person out in one of the groups
# So we might have a group of 3
# If we have an odd number of people that is not 1,
# or if there is 1 person left unpaired:
# We put the odd person out in one of the groups
# So we might have a group of 3

if len(not_paired) == 1 and len(pairs) > 0:
pairs[len(pairs) - 1].append(not_paired[0])
# In the case the algorithm failed, purge pairings and repeat
elif len(not_paired) > 1:
logging.info("Failed to pair users, purging and trying again..")
self.__purge_pairings()
return self.__pair_users(users)
logging.info("Done pairing algorithm")
return pairs

def __get_previous_pairs(self, user: str) -> Set[str]:
logging.info(f"Getting previous pairs for {user}")
pairings = self.facade.query_or(Pairing,
[('user1_slack_id', user),
('user2_slack_id', user)])
res = set()
for pairing in pairings:
other = pairing.user1_slack_id if pairing.user2_slack_id == user \
else pairing.user2_slack_id
res.add(other)
logging.info(f"Previous pairings are {res}")
return res

def __persist_pairing(self, user1_slack_id: str, user2_slack_id: str):
pairing = Pairing(user1_slack_id, user2_slack_id)
reverse_pairing = Pairing(user2_slack_id, user1_slack_id)
self.facade.store(pairing)
self.facade.store(reverse_pairing)

def __purge_pairings(self):
logging.info("Deleting all pairings")
self.facade.delete_all(Pairing)
7 changes: 2 additions & 5 deletions app/server.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
"""Flask server instance."""
from factory import make_command_parser, make_github_webhook_handler, \
make_slack_events_handler, make_github_interface
make_slack_events_handler, make_github_interface, make_event_scheduler
from flask import Flask, request
from logging.config import dictConfig
from slackeventsapi import SlackEventAdapter
from apscheduler.schedulers.background import BackgroundScheduler
import logging
import structlog
from flask_talisman import Talisman
from config import Config
from app.scheduler import Scheduler
from interface.slack import Bot
from slack import WebClient
from boto3.session import Session
Expand Down Expand Up @@ -81,8 +79,7 @@
slack_events_adapter = SlackEventAdapter(config.slack_signing_secret,
"/slack/events",
app)
sched = Scheduler(BackgroundScheduler(timezone="America/Los_Angeles"),
(app, config))
sched = make_event_scheduler(app, config)
sched.start()

bot = Bot(WebClient(config.slack_api_token),
Expand Down
11 changes: 9 additions & 2 deletions config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ class Config:
'SLACK_API_TOKEN': 'slack_api_token',
'SLACK_NOTIFICATION_CHANNEL': 'slack_notification_channel',
'SLACK_ANNOUNCEMENT_CHANNEL': 'slack_announcement_channel',

'SLACK_PAIRING_CHANNEL': 'slack_pairing_channel',
tarikeshaq marked this conversation as resolved.
Show resolved Hide resolved
'SLACK_PAIRING_FREQUENCY': 'slack_pairing_frequency',
'GITHUB_APP_ID': 'github_app_id',
'GITHUB_ORG_NAME': 'github_org_name',
'GITHUB_DEFAULT_TEAM_NAME': 'github_team_all',
Expand All @@ -31,6 +32,7 @@ class Config:
'AWS_SECRET_KEY': 'aws_secret_key',
'AWS_USERS_TABLE': 'aws_users_tablename',
'AWS_TEAMS_TABLE': 'aws_teams_tablename',
'AWS_PAIRINGS_TABLE': 'aws_pairings_tablename',
'AWS_PROJECTS_TABLE': 'aws_projects_tablename',
'AWS_REGION': 'aws_region',
'AWS_LOCAL': 'aws_local',
Expand All @@ -45,6 +47,9 @@ class Config:
'GITHUB_LEADS_TEAM_NAME': '',
'GCP_SERVICE_ACCOUNT_CREDENTIALS': '',
'GCP_SERVICE_ACCOUNT_SUBJECT': '',
'SLACK_PAIRING_CHANNEL': '',
'AWS_PAIRINGS_TABLE': 'pairings',
'SLACK_PAIRING_FREQUENCY': '',
}

def __init__(self):
Expand Down Expand Up @@ -89,7 +94,8 @@ def _set_attrs(self):
self.slack_api_token = ''
self.slack_notification_channel = ''
self.slack_announcement_channel = ''

self.slack_pairing_channel = ''
self.slack_pairing_frequency = ''
self.github_app_id = ''
self.github_org_name = ''
self.github_team_all = ''
Expand All @@ -103,6 +109,7 @@ def _set_attrs(self):
self.aws_secret_key = ''
self.aws_users_tablename = ''
self.aws_teams_tablename = ''
self.aws_pairings_tablename = ''
self.aws_projects_tablename = ''
self.aws_region = ''
self.aws_local: bool = False
Expand Down
Loading