add pylint so the code has more hygeine (#34)

.gitignore

@@ -1,3 +1,4 @@
 py36*
 audit.log
 whitelist.txt
+__pycache__

.travis.yml

@@ -1,8 +1,10 @@
 language: python
 python:
-  - "2.7"
+  - "3.6"
 install:
   - pip install -r requirements-dev.txt
 # commands to run, must exit 0 to pass
-script: flake8
-script: yapf --parallel --verbose --recursive --diff .
+script:
+  - flake8
+  - yapf --parallel --verbose --recursive --diff .
+  - pylint --max-line-length=120 *.py

config.py

@@ -0,0 +1,24 @@
+"""
+All settings you can change for running slack channel reaper will live in this file.
+"""
+
+import os
+from datetime import datetime, timedelta
+
+
+def get_channel_reaper_settings():
+    """ This returns a dictionary of all settings. """
+    days_inactive = int(os.environ.get('DAYS_INACTIVE', 60))
+    return {
+        'admin_channel': os.environ.get('ADMIN_CHANNEL', ''),
+        'days_inactive': days_inactive,
+        # set MIN_MEMBERS and any channels larger than this in people
+        # are exempt from archiving. 0 is no limit.
+        'min_members': int(os.environ.get('MIN_MEMBERS', 0)),
+        'dry_run': (os.environ.get('DRY_RUN', 'true') == 'true'),
+        'slack_token': os.environ.get('SLACK_TOKEN', ''),
+        'too_old_datetime': (datetime.now() - timedelta(days=days_inactive)),
+        'whitelist_keywords': os.environ.get('WHITELIST_KEYWORDS', ''),
+        'skip_subtypes': {'channel_leave', 'channel_join'},
+        'skip_channel_str': os.environ.get('SLACK_SKIP_PURPOSE', '%noarchive'),
+    }

slack-autoarchive.py → slack_autoarchive.py

@@ -1,70 +1,74 @@
-#!/usr/bin/python3.6
+#!/usr/bin/env python
+"""
+This program lets you do archive slack channels which are no longer active.
+"""
 
-from datetime import timedelta, datetime
-import logging
+# standard imports
+from datetime import datetime
 import os
-import requests
 import sys
 import time
 import json
 
-log_file = 'audit.log'
-logging.basicConfig(filename=log_file,
-                    level=logging.INFO,
-                    format='%(asctime)s - %(levelname)s - %(message)s')
+# not standard imports
+import requests
+from config import get_channel_reaper_settings
+from utils import get_logger
 
-#
-# This will archive inactive channels. The inactive period is in days as 'self.days_inactive'
-# You can put this in a cron job to run daily to do slack cleanup.
-#
 
+class ChannelReaper():
+    """
+    This class can be used to archive slack channels.
+    """
 
-class ChannelReaper(object):
     def __init__(self):
-        self.admin_channel = os.getenv('ADMIN_CHANNEL')
-        self.days_inactive = int(os.getenv('DAYS_INACTIVE', 60))
-        # set MIN_MEMBERS and any channels larger than this in people
-        # are exempt from archiving. 0 is no limit.
-        self.min_members = int(os.getenv('MIN_MEMBERS', 0))
-        self.dry_run = (os.getenv('DRY_RUN', 'true') == 'true')
-        self.slack_token = os.getenv('SLACK_TOKEN')
-        self.too_old_datetime = datetime.now() - timedelta(
-            days=self.days_inactive)
-        self.whitelist_keywords = os.getenv('WHITELIST_KEYWORDS')
-        self.skip_subtypes = {'channel_leave', 'channel_join'}  # 'bot_message'
-        # note, if the channel purpose has this string in it, we'll skip archiving this channel.
-        self.skip_channel_str = os.getenv('SLACK_SKIP_PURPOSE', '%noarchive')
+        self.settings = get_channel_reaper_settings()
+        self.logger = get_logger('channel_reaper', './audit.log')
 
     def get_whitelist_keywords(self):
+        """
+        Get all whitelist keywords. If this word is used in the channel
+        purpose or topic, this will make the channel exempt from archiving.
+        """
         keywords = []
         if os.path.isfile('whitelist.txt'):
-            with open('whitelist.txt') as f:
-                keywords = f.readlines()
+            with open('whitelist.txt') as filecontent:
+                keywords = filecontent.readlines()
 
         # remove whitespace characters like `\n` at the end of each line
         keywords = map(lambda x: x.strip(), keywords)
-        if self.whitelist_keywords:
-            keywords = keywords + self.whitelist_keywords.split(',')
+        whitelist_keywords = self.settings.get('whitelist_keywords')
+        if whitelist_keywords:
+            keywords = keywords + whitelist_keywords.split(',')
         return list(keywords)
 
     def get_channel_alerts(self):
-        archive_msg = "This channel has had no activity for %d days. It is being auto-archived. If you feel this is a mistake you can <https://get.slack.help/hc/en-us/articles/201563847-Archive-a-channel#unarchive-a-channel|unarchive this channel> to bring it back at any point. In the future, you can add '%%noarchive' to your channel topic or purpose to avoid being archived. This script was run from this repo: https://github.com/Symantec/slack-autoarchive" % self.days_inactive
+        """Get the alert message which is used to notify users in a channel of archival. """
+        archive_msg = """
+This channel has had no activity for %d days. It is being auto-archived.
+If you feel this is a mistake you can <https://get.slack.help/hc/en-us/articles/201563847-Archive-a-channel#unarchive-a-channel|unarchive this channel>.
+This will bring it back at any point. In the future, you can add '%%noarchive' to your channel topic or purpose to avoid being archived.
+This script was run from this repo: https://github.com/Symantec/slack-autoarchive
+""" % self.settings.get('days_inactive')
         alerts = {'channel_template': archive_msg}
         if os.path.isfile('templates.json'):
-            with open('templates.json') as f:
-                alerts = json.load(f)
+            with open('templates.json') as filecontent:
+                alerts = json.load(filecontent)
         return alerts
 
-    # api_endpoint is a string, and payload is a dict
-    def slack_api_http(self,
-                       api_endpoint=None,
-                       payload=None,
-                       method='GET',
-                       retry=True,
-                       retry_delay=0):
-
+    # pylint: disable=too-many-arguments
+    def slack_api_http(
+            self,
+            api_endpoint=None,
+            payload=None,
+            method='GET',
+            # pylint: disable=unused-argument
+            retry=True,
+            retry_delay=0):
+        """ Helper function to query the slack api and handle errors and rate limit. """
+        # pylint: disable=no-member
         uri = 'https://slack.com/api/' + api_endpoint
-        payload['token'] = self.slack_token
+        payload['token'] = self.settings.get('slack_token')
         try:
             # Force request to take at least 1 second. Slack docs state:
             # > In general we allow applications that integrate with Slack to send
@@ -80,7 +84,7 @@ def slack_api_http(self,
 
             if response.status_code == requests.codes.ok and 'error' in response.json(
             ) and response.json()['error'] == 'not_authed':
-                print(
+                self.logger.error(
                     'Need to setup auth. eg, SLACK_TOKEN=<secret token> python slack-autoarchive.py'
                 )
                 sys.exit(1)
@@ -89,15 +93,15 @@ def slack_api_http(self,
                 return response.json()
             elif response.status_code == requests.codes.too_many_requests:
                 retry_timeout = float(response.headers['Retry-After'])
+                # pylint: disable=too-many-function-args
                 return self.slack_api_http(api_endpoint, payload, method,
                                            False, retry_timeout)
-            else:
-                raise
         except Exception as error_msg:
             raise Exception(error_msg)
+        return None
 
-    # too_old_datetime is a datetime object
     def get_all_channels(self):
+        """ Get a list of all non-archived channels from slack channels.list. """
         payload = {'exclude_archived': 1}
         api_endpoint = 'channels.list'
         channels = self.slack_api_http(api_endpoint=api_endpoint,
@@ -113,6 +117,7 @@ def get_all_channels(self):
         return all_channels
 
     def get_last_message_timestamp(self, channel_history, too_old_datetime):
+        """ Get the last message from a slack channel, and return the time. """
         last_message_datetime = too_old_datetime
         last_bot_message_datetime = too_old_datetime
 
@@ -121,7 +126,7 @@ def get_last_message_timestamp(self, channel_history, too_old_datetime):
 
         for message in channel_history['messages']:
             if 'subtype' in message and message[
-                    'subtype'] in self.skip_subtypes:
+                    'subtype'] in self.settings.get('skip_subtypes'):
                 continue
             last_message_datetime = datetime.fromtimestamp(float(
                 message['ts']))
@@ -131,12 +136,12 @@ def get_last_message_timestamp(self, channel_history, too_old_datetime):
         if not last_message_datetime:
             last_bot_message_datetime = datetime.utcfromtimestamp(0)
         # return bot message time if there was no user message
-        if last_bot_message_datetime > too_old_datetime and last_message_datetime <= too_old_datetime:
+        if too_old_datetime >= last_bot_message_datetime > too_old_datetime:
             return (last_bot_message_datetime, False)
-        else:
-            return (last_message_datetime, True)
+        return (last_message_datetime, True)
 
     def is_channel_disused(self, channel, too_old_datetime):
+        """ Return True or False depending on if a channel is "active" or not.  """
         num_members = channel['num_members']
         payload = {'inclusive': 0, 'oldest': 0, 'count': 50}
         api_endpoint = 'channels.history'
@@ -149,22 +154,25 @@ def is_channel_disused(self, channel, too_old_datetime):
         # mark inactive if last message is too old, but don't
         # if there have been bot messages and the channel has
         # at least the minimum number of members
-        has_min_users = (self.min_members == 0
-                         or self.min_members > num_members)
+        min_members = self.settings.get('min_members')
+        has_min_users = (min_members == 0 or min_members > num_members)
         return last_message_datetime <= too_old_datetime and (not is_user
                                                               or has_min_users)
 
     # If you add channels to the WHITELIST_KEYWORDS constant they will be exempt from archiving.
     def is_channel_whitelisted(self, channel, white_listed_channels):
-        # self.skip_channel_str
-        # if the channel purpose contains the string self.skip_channel_str, we'll skip it.
+        """ Return True or False depending on if a channel is exempt from being archived. """
+        # self.settings.get('skip_channel_str')
+        # if the channel purpose contains the string self.settings.get('skip_channel_str'), we'll skip it.
         info_payload = {'channel': channel['id']}
         channel_info = self.slack_api_http(api_endpoint='channels.info',
                                            payload=info_payload,
                                            method='GET')
         channel_purpose = channel_info['channel']['purpose']['value']
         channel_topic = channel_info['channel']['topic']['value']
-        if self.skip_channel_str in channel_purpose or self.skip_channel_str in channel_topic:
+        if self.settings.get(
+                'skip_channel_str') in channel_purpose or self.settings.get(
+                    'skip_channel_str') in channel_topic:
             return True
 
         # check the white listed channels (file / env)
@@ -175,6 +183,7 @@ def is_channel_whitelisted(self, channel, white_listed_channels):
         return False
 
     def send_channel_message(self, channel_id, message):
+        """ Send a message to a channel or user. """
         payload = {
             'channel': channel_id,
             'username': 'channel_reaper',
@@ -187,31 +196,37 @@ def send_channel_message(self, channel_id, message):
                             method='POST')
 
     def archive_channel(self, channel, alert):
+        """ Archive a channel, and send alert to slack admins. """
         api_endpoint = 'channels.archive'
         stdout_message = 'Archiving channel... %s' % channel['name']
-        print(stdout_message)
+        self.logger.info(stdout_message)
 
-        if not self.dry_run:
-            channel_message = alert.format(self.days_inactive)
+        if not self.settings.get('dry_run'):
+            channel_message = alert.format(self.settings.get('days_inactive'))
             self.send_channel_message(channel['id'], channel_message)
             payload = {'channel': channel['id']}
             self.slack_api_http(api_endpoint=api_endpoint, payload=payload)
-            logging.info(stdout_message)
+            self.logger.info(stdout_message)
 
     def send_admin_report(self, channels):
-        if self.admin_channel:
+        """ Optionally this will message admins with which channels were archived. """
+        if self.settings.get('admin_channel'):
             channel_names = ', '.join('#' + channel['name']
                                       for channel in channels)
             admin_msg = 'Archiving %d channels: %s' % (len(channels),
                                                        channel_names)
-            if self.dry_run:
+            if self.settings.get('dry_run'):
                 admin_msg = '[DRY RUN] %s' % admin_msg
-            self.send_channel_message(self.admin_channel, admin_msg)
+            self.send_channel_message(self.settings.get('admin_channel'),
+                                      admin_msg)
 
     def main(self):
-
-        if self.dry_run:
-            print('THIS IS A DRY RUN. NO CHANNELS ARE ACTUALLY ARCHIVED.')
+        """
+        This is the main method that checks all inactive channels and archives them.
+        """
+        if self.settings.get('dry_run'):
+            self.logger.info(
+                'THIS IS A DRY RUN. NO CHANNELS ARE ACTUALLY ARCHIVED.')
 
         whitelist_keywords = self.get_whitelist_keywords()
         alert_templates = self.get_channel_alerts()
@@ -221,9 +236,11 @@ def main(self):
             sys.stdout.write('.')
             sys.stdout.flush()
 
-            if (not self.is_channel_whitelisted(channel, whitelist_keywords)
-                    and self.is_channel_disused(channel,
-                                                self.too_old_datetime)):
+            channel_whitelisted = self.is_channel_whitelisted(
+                channel, whitelist_keywords)
+            channel_disused = self.is_channel_disused(
+                channel, self.settings.get('too_old_datetime'))
+            if (not channel_whitelisted and channel_disused):
                 archived_channels.append(channel)
                 self.archive_channel(channel,
                                      alert_templates['channel_template'])
@@ -232,5 +249,5 @@ def main(self):
 
 
 if __name__ == '__main__':
-    channel_reaper = ChannelReaper()
-    channel_reaper.main()
+    CHANNEL_REAPER = ChannelReaper()
+    CHANNEL_REAPER.main()

tox.ini

@@ -9,5 +9,6 @@ exclude = .git,venv
 ; E231 is missing whitespace after  ,/:
 ; E251 is unexpected spaces around keywords
 ; E501 is lines too long -- unfortunately this triggers on long strings like SQL queries
-ignore = E111,E114,E121,E128,E203,E221,E231,E251,E401,E501,D101,D100,D103
-max-line-length = 100
+; W503 is line break before binary operator
+ignore = E111,E114,E121,E128,E203,E221,E231,E251,E401,E501,D101,D100,D103,W503
+max-line-length = 120

utils.py

@@ -0,0 +1,24 @@
+""" Helper functions that don't quite belong in the slack_autoarchive class """
+
+import logging
+
+
+def get_logger(logger_name, logger_file, log_level=logging.INFO):
+    """ Setup the logger and return it. """
+    log_format = '%(asctime)s - %(levelname)s - %(message)s'
+    logging.basicConfig(level=log_level,
+                        format=log_format,
+                        datefmt='%y-%m-%d_%H:%M',
+                        filename=logger_file,
+                        filemode='w')
+    # define a Handler which writes INFO messages or higher to the sys.stderr
+    console = logging.StreamHandler()
+    console.setLevel(log_level)
+    # set a format which is simpler for console use
+    formatter = logging.Formatter(log_format)
+    # tell the handler to use this format
+    console.setFormatter(formatter)
+    # add the handler to the root logger
+    logging.getLogger(logger_name).addHandler(console)
+
+    return logging.getLogger(logger_name)