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

docs: general formatting, fix links, cleanups, rewrites #554

Merged
merged 10 commits into from
Oct 13, 2020
4 changes: 2 additions & 2 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Before You Open a Pull Request
- Your code is presentable and you have **not** committed extra files
(think your credentials, IDE config files, cached directories, build
directories, etc.)
- Youve written unit tests for the changes youve made, and that they
- You've written unit tests for the changes you've made, and that they
cover all the code you wrote (or effectively all, given the
circumstances)

Expand Down Expand Up @@ -123,7 +123,7 @@ Assuming you are on your working branch:
git rebase master

If you have changed files that were also changed in the intervening
merge, ``git rebase`` may report merge conflicts. If this happens, dont
merge, ``git rebase`` may report merge conflicts. If this happens, don't
panic! Use ``git status`` and ``git diff`` to determine which files
conflict and where, use an editor to fix the conflicts, then stage the
formerly-conflicting files with ``git add FILE``. Finally, use
Expand Down
6 changes: 5 additions & 1 deletion app/controller/command/commands/karma.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ def __init__(self, db_facade):
self.help = self.get_help()

def init_subparsers(self) -> _SubParsersAction:
"""Initialize subparsers for karma command."""
"""
Initialize subparsers for karma command.

:meta private:
"""
subparsers: _SubParsersAction = \
self.parser.add_subparsers(dest="which")

Expand Down
8 changes: 4 additions & 4 deletions app/controller/command/commands/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ def unassign_helper(self,

:param project_id: project ID of project to unassign team from
:param user_id: user ID of the calling user
:return: returns lookup error if the project, assigned team or calling
:return: error string if the project, assigned team or calling
user could not be found, else permission error if calling
user is not a team lead of the team to unassign,
otherwise success message
Expand Down Expand Up @@ -321,7 +321,7 @@ def edit_helper(self,

:param project_id: project ID of the project in the database to edit
:param param_list: Dict of project parameters that are to be edited
:return: returns edit message if project is successfully edited, or an
:return: edit message if project is successfully edited, or an
error message if the project was not found in the database
"""
logging.debug("Handling project edit subcommand")
Expand Down Expand Up @@ -354,7 +354,7 @@ def assign_helper(self,
:param user_id: user ID of the calling user
:param force: specify if an error should be raised if the project
is assigned to another team
:return: returns lookup error if the project could not be found or no
:return: error string if the project could not be found or no
team has the specified GitHub team name or if the calling
user is not in the database, else permission error if calling
user is not a team lead, else an assignment error if the
Expand Down Expand Up @@ -404,7 +404,7 @@ def delete_helper(self,
:param user_id: user ID of the calling user
:param force: specify if an error should be raised if the project
is assigned to a team
:return: returns lookup error if the project, assigned team, or user
:return: error string if the project, assigned team, or user
could not be found, else an assignment error if the project
is assigned to a team, otherwise success message
"""
Expand Down
11 changes: 8 additions & 3 deletions app/controller/command/commands/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def __init__(self,
:param db_facade: Given Dynamo_DB Facade
:param gh: Given Github Interface
:param sc: Given Slack Client Interface
"param gcp: Given GCP client
:param gcp: Given GCP client
"""
logging.info("Initializing TeamCommand instance")
self.facade = db_facade
Expand All @@ -55,7 +55,11 @@ def __init__(self,
self.help = self.get_help()

def init_subparsers(self) -> _SubParsersAction:
"""Initialize subparsers for team command."""
"""
Initialize subparsers for team command.

:meta private:
"""
subparsers = self.parser.add_subparsers(dest="which")

"""Parser for list command."""
Expand Down Expand Up @@ -635,7 +639,8 @@ def refresh_helper(self, user_id) -> ResponseTuple:
Ensure that the local team database is the same as GitHub's.

In the event that our local team database is outdated compared to
the teams on GitHub, this command can be called to fix things.
the teams on GitHub, this command can be called to fix these
inconsistencies.
Copy link
Member

Choose a reason for hiding this comment

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

This likely ties into #490 in that we can't remove this command, as we'll always need something to forcibly bring everything in line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably need to add a Deprecation Warning whenever someone calls the command.

Copy link
Member

Choose a reason for hiding this comment

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

Oh my point was I dont think we can deprecate it, ever, since we need some way to force a state sync (for example, to add new features - essentially a "migration")


:return: error message if user has insufficient permission level
otherwise returns success messages with # of teams changed
Expand Down
10 changes: 7 additions & 3 deletions app/controller/command/commands/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ def __init__(self,
self.gcp = gcp

def init_subparsers(self) -> _SubParsersAction:
"""Initialize subparsers for user command."""
"""
Initialize subparsers for user command.

:meta private:
"""
subparsers = self.parser.add_subparsers(dest="which")

# Parser for view command
Expand Down Expand Up @@ -223,8 +227,8 @@ def edit_helper(self,

:param user_id: Slack ID of user who is calling the command
:param param_list: List of user parameters that are to be edited
:return: returns error message if not admin and command
edits another user, returns edit message if user is edited
:return: error message if not admin and command edits another user,
or the edit message if user is edited
"""
is_admin = False
edited_user = None
Expand Down
2 changes: 1 addition & 1 deletion app/controller/webhook/github/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def verify_hash(self, request_body: bytes, xhub_signature: str):

:param request_body: Byte string of the request body
:param xhub_signature: Hashed signature to validate
:return: Return True if the signature is valid, False otherwise
:return: True if the signature is valid, False otherwise
"""
h = hmac.new(bytes(self.__secret, encoding='utf8'),
request_body, hashlib.sha1)
Expand Down
4 changes: 2 additions & 2 deletions app/model/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def from_dict(cls: Type[T], d: Dict[str, Any]) -> T:
Convert dict response object to data model object.

:param d: the dictionary representing a data model
:return: returns converted data model object.
:return: the converted data model object.
"""
pass

Expand All @@ -44,6 +44,6 @@ def is_valid(cls: Type[T], model: T) -> bool:
Return true if this data model has no missing required fields.

:param model: data model object to check
:return: return true if this data model has no missing required fields
:return: true if this data model has no missing required fields
"""
pass
4 changes: 2 additions & 2 deletions app/model/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def from_dict(cls: Type[T], d: Dict[str, Any]) -> T:
Convert dict response object to team model.

:param d: the dictionary representing a team
:return: returns converted team model.
:return: the converted team model.
"""
team = cls(d['github_team_id'],
d['github_team_name'],
Expand Down Expand Up @@ -113,7 +113,7 @@ def is_valid(cls: Type[T], team: T) -> bool:
- ``github_team_id``

:param team: team to check
:return: returns true if this team has no missing required fields
:return: true if this team has no missing required fields
"""
return len(team.github_team_name) > 0 and\
len(team.github_team_id) > 0
Expand Down
4 changes: 2 additions & 2 deletions app/model/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def from_dict(cls: Type[T], d: Dict[str, Any]) -> T:
Convert dict response object to user model.

:param d: the dictionary representing a user
:return: returns converted user model.
:return: the converted user model.
"""
user = cls(d['slack_id'])
user.email = d.get('email', '')
Expand All @@ -110,7 +110,7 @@ def is_valid(cls: Type[T], user: T) -> bool:
- ``permissions_level``

:param user: user to check
:return: return true if this user has no missing required fields
:return: true if this user has no missing required fields
"""
return len(user.slack_id) > 0

Expand Down
8 changes: 6 additions & 2 deletions conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
author = 'UBC Launch Pad'

# The short X.Y version
version = '1.0'
version = '2.0'
# The full version, including alpha/beta/rc tags
release = '1.0.0'
release = '2.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

do we even do releases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question. Actually, there is a slim chance that this would come back and bite us. Technically we do releases. We are currently on v2.1.0. I'm thinking that'd be one of the things we'd talk about in the meeting?

Gonna change this to better reflect, but right now docs are generated (and not saved) on master push, soooo rolling release.

Copy link
Member

Choose a reason for hiding this comment

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

I dropped an item in the meeting notes - feel free to add stuff! https://docs.google.com/document/d/1KcH2l6nJKyHoCDe6v6Sl0cLWQWKQQmSLqk_kXvZUsRw/edit



# -- General configuration ---------------------------------------------------
Expand All @@ -42,6 +42,10 @@
'sphinx_autodoc_typehints',
]

# Config options for the extensions
typehints_fully_qualified = True
always_document_param_types = True

# Add any paths that contain templates here, relative to this directory.
templates_path = ['docs/_templates']

Expand Down
2 changes: 1 addition & 1 deletion config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def __init__(self, missing_config_fields):
"""
Initialize a new MissingConfigError.

:param: missing_config_fields List of missing config variables
:param missing_config_fields: the missing config variables
Copy link
Member

Choose a reason for hiding this comment

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

is there a linter that can catch these documentation errors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I doubt it.

"""
self.error = 'Please set the following env variables:\n' + \
'\n'.join(missing_config_fields)
42 changes: 24 additions & 18 deletions db/dynamodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ class DynamoDB(DBFacade):
"""

class Const:
"""A bunch of static constants and functions."""
"""
A bunch of static constants and functions.

Used to convert between Python objects and the DDB table names, object
attributes and table column names.
"""

def __init__(self, config: Config):
"""Initialize the constants."""
Expand All @@ -50,7 +55,7 @@ def get_table_name(self, cls: Type[T]) -> str:
Convert class into corresponding table name.

:param cls: Either ``User``, ``Team``, or ``Project``
:raise: TypeError if it is not either User, Team, or Project
:raises: TypeError if it is not either User, Team, or Project
:return: table name string
"""
if cls == User:
Expand All @@ -67,7 +72,7 @@ def get_key(self, table_name: str) -> str:
Get primary key of the table name.

:param cls: the name of the table
:raise: TypeError if table does not exist
:raises: TypeError if table does not exist
:return: primary key of the table
"""
if table_name == self.users_table:
Expand All @@ -84,8 +89,8 @@ def get_set_attrs(self, table_name: str) -> List[str]:
Get class attributes that are sets.

:param cls: the table name
:raise: TypeError if table does not exist
:return: list of strings of set attributes
:raises: TypeError if table does not exist
:return: set attributes
"""
if table_name == self.users_table:
return []
Expand All @@ -97,21 +102,22 @@ def get_set_attrs(self, table_name: str) -> List[str]:
raise TypeError('Table name does not correspond to anything')

def __init__(self, config: Config):
"""Initialize facade using DynamoDB settings.
"""
Initialize facade using DynamoDB settings.

To avoid local tests failure when the DynamoDb server is used,
a testing environment variable is set.
When testing environmental variable is true,
the local dynamodb is run.
When testing environmental variable is true,
the server dynamodb is run.

boto3.resource() takes in a service_name, region_name, and endpoint_url
(only for local dynamodb).
service_name: The name of a service, "dynamodb" in this case.
region_name: The name of the region associated with the client.
A list of different regions can be obtained online.
endpoint_url: The complete URL to use for the constructed client.
an environment variable ``config.aws_local`` is read.

.. code:: python

if config.aws_local:
# Connect to locally-run instance of DynamoDB
pass
else:
# Connect to remote instance of DynamoDB
pass

:param config: configuration used to initialize
"""
logging.info("Initializing DynamoDb")
self.users_table = config.aws_users_tablename
Expand Down
6 changes: 3 additions & 3 deletions db/facade.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def store(self, obj: T) -> bool:
"""
Store object into the correct table.

Object can be of type :class:`model.user.User`,
:class:`model.team.Team`, or :class:`model.project.Project`.
Object can be of type :class:`app.model.user.User`,
:class:`app.model.team.Team`, or :class:`app.model.project.Project`.

:param obj: Object to store in database
:return: True if object was stored, and false otherwise
Expand All @@ -38,7 +38,7 @@ def retrieve(self, Model: Type[T], k: str) -> T:

:param Model: the actual class you want to retrieve
:param k: retrieve based on this key (or ID)
:raise: LookupError if key is not found
:raises: LookupError if key is not found
:return: a model ``Model`` if key is found
"""
raise NotImplementedError
Expand Down
28 changes: 16 additions & 12 deletions docs/Architecture.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@ Architecture

.. image:: rocket2arch.png

**What is the ``db`` module?** The database ``db`` module consists of
the ``facade`` and the ``dynamodb`` database we are using.
Our Flask server serves to handle all incoming Slack events, slash command, and
Github webhooks. Slash commands are handled by :py:mod:`app.controller.command.parser`,
and the remaining events and webhooks are handled by :py:mod:`app.controller.webhook.github.core`
and :py:mod:`app.controller.webhook.slack.core`.

**What is the ``command`` module?** The ``command`` module is where the
slack commands get parsed and passed on to the backend so models can be
created and the database can be populated.
We store our data in an Amazon DynamoDB, which can be accessed directly by the
database facade :py:class:`db.dynamodb.DynamoDB`.

**What is the ``model`` module?** The ``model`` module is where models
are constructed. Currently we have ``Team`` and ``User`` models.
We treat Github itself as the sole source of truth. Any modifications done on the
chuck-sys marked this conversation as resolved.
Show resolved Hide resolved
Github side (e.g. changing team names, adding/removing team members,
creating/deleting teams, etc.) is reflected into the database.
Whenever an attempt is made at modifying the existing teams (e.g. using a slash
command to add/remove members, create/delete teams, edit teams), the changes are
made using the Github API, and then done on our database.

**How do ``db``, ``command``, ``model`` modules interact with each
other?** First a command is input through slack. Then, the input will be
parsed so a model can be populated. After the model gets populated, the
model can then be added into the db. The db contains a separate table
for each type of model.
We run a cron-style scheduler that execute specific tasks at regular intervals.
To learn how to add tasks and modules to it, have a look at `this tutorial`_.

.. _this tutorial: DevelopmentTutorials.html#create-a-scheduler-module
Loading