Skip to content

Commit e230d98

Browse files
committed
Improved Python code style by fixing all Flake8 linting warnings
1 parent fd9fe43 commit e230d98

21 files changed

+122
-66
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
**Improved:**
66

77
- Updated docker-compose `policyd` service Docker image from `python:3.8` to `python:3.11`
8+
- Improved Python code style by fixing all Flake8 linting warnings.
89

910
**Added:**
1011

README.md

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ $ git clone https://github.com/onlime/policyd-rate-guard.git
9696

9797
```bash
9898
$ cd /opt/policyd-rate-guard
99-
$ python3 -m venv venv
100-
$ . venv/bin/activate
99+
$ python3 -m venv .venv
100+
$ . .venv/bin/activate
101101
(venv)$ pip install --upgrade pip
102102
(venv)$ pip install -r requirements.txt
103103
(venv)$ cp yoyo.ini.example yoyo.ini # & Adjust the settings
@@ -389,8 +389,8 @@ To set up a basic developer environment, run the following commands:
389389
```bash
390390
$ git clone git@gitlab.onlime.ch:onlime/policyd-rate-guard.git
391391
$ cd policyd-rate-guard
392-
$ python3 -m venv venv
393-
$ . venv/bin/activate
392+
$ python3 -m venv .venv
393+
$ . .venv/bin/activate
394394
(venv)$ pip install --upgrade pip
395395
(venv)$ pip install -r requirements.txt
396396
$ docker-compose up -d db
@@ -487,6 +487,18 @@ $ . venv/bin/activate
487487
> [!IMPORTANT]
488488
> Make sure to always run the tests inside your venv!
489489
490+
### Linting with flake8
491+
492+
You can run Python code style linting with [Flake8](https://flake8.pycqa.org/) locally in your venv:
493+
494+
```bash
495+
$ . venv/bin/activate
496+
(venv)$ pip install flake8
497+
(venv)$ flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics --exclude=venv
498+
```
499+
500+
The same will also be done in our [Github workflow `ci.yml`](https://github.com/onlime/policyd-rate-guard/actions/workflows/ci.yml).
501+
490502
### Configure Sentry SDK
491503

492504
Sentry integration can be both used in your development environment and on production.

app/conf.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
from distutils.util import strtobool
44
from typing import Any
55

6+
67
class Config:
7-
def __init__(self, path = None) -> None:
8+
def __init__(self, path: str = None) -> None:
89
load_dotenv(dotenv_path=path)
910

1011
def get(self, key: str, default: Any = None) -> str:
@@ -15,4 +16,4 @@ def get(self, key: str, default: Any = None) -> str:
1516
return value
1617

1718
def get_array(self, key: str, default: Any = None) -> list:
18-
return getenv(key, default).split(',')
19+
return getenv(key, default).split(',')

app/db.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
11
from importlib import import_module
22
from dbutils.pooled_db import PooledDB
33

4+
45
class DbConnectionPool:
56
def __init__(self, conf: object):
67
self.driver = conf.get('DB_DRIVER', 'pymysql').lower()
78
self.backend = import_module(self.driver)
89
db_config = getattr(self, 'get_dbconfig_' + self.driver)(conf)
910

1011
self.pool = PooledDB(
11-
creator=self.backend, # pymysql or sqlite3
12+
creator=self.backend, # pymysql or sqlite3
1213
mincached=int(conf.get('DB_POOL_MINCACHED', 0)),
1314
maxcached=int(conf.get('DB_POOL_MAXCACHED', 10)),
1415
maxshared=int(conf.get('DB_POOL_MAXSHARED', 10)),
1516
maxusage=int(conf.get('DB_POOL_MAXUSAGE', 10000)),
1617
# maxconnections=int(conf.get('DB_POOL_MAXCONNECTIONS', 20)),
1718
**db_config
1819
)
19-
20+
2021
def connection(self):
2122
connection = self.pool.connection()
2223
if self.driver == 'sqlite3':
2324
# https://docs.python.org/3/library/sqlite3.html#sqlite3.Row
2425
connection.row_factory = self.backend.Row
2526
return connection
2627

27-
2828
def get_dbconfig_pymysql(self, conf: object) -> dict:
2929
return {
3030
'host': conf.get('DB_HOST', 'localhost'),

app/handler.py

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from .message import Message
22

3+
34
class Handler:
45
"""Handle request"""
56

@@ -14,21 +15,24 @@ def __init__(self, conn: object, addr: str, conf: object, logger: object, db_poo
1415
self.db_pool = db_pool
1516
try:
1617
self.handle()
17-
except Exception as e: # pragma: no cover
18+
except Exception as e: # pragma: no cover
1819
self.logger.exception('Unhandled Exception: %s', e)
1920
self.logger.warning('Received DATA: %s', self.data)
20-
self.send_response('DUNNO') # use DUNNO as accept action, just to distinguish between OK and unhandled exception
21+
self.send_response('DUNNO') # use DUNNO as accept action, just to distinguish between OK and unhandled exception
2122

2223
def handle(self) -> None:
2324
"""Handle request"""
2425
# Read data
25-
self.data = self.conn.recv(2048).decode('utf-8') # Attention: We only read first 2048 bytes, which is sufficient for our needs
26+
# Attention: We only read first 2048 bytes, which is sufficient for our needs
27+
self.data = self.conn.recv(2048).decode('utf-8')
2628
if not self.data:
2729
raise Exception('No data received')
2830
self.logger.debug('Received data: %s', self.data)
2931

3032
# Parse data using a dictionary comprehension
31-
request = { key: value for line in self.data.strip().split("\n") for key, value in [line.strip().split('=', 1)] if value }
33+
request = {
34+
key: value for line in self.data.strip().split("\n") for key, value in [line.strip().split('=', 1)] if value
35+
}
3236
self.logger.debug('Parsed request: %s', request)
3337

3438
# Add msgid to logger
@@ -60,17 +64,18 @@ def handle(self) -> None:
6064
)
6165
message.get_ratelimit()
6266
was_blocked = message.is_blocked()
63-
message.update_ratelimit() # Always update counter, even if quota limit was already reached before (was_blocked)!
64-
blocked = message.is_blocked() # ... and make sure we check for blocked after having raised the counter.
67+
message.update_ratelimit() # Always update counter, even if quota limit was already reached before (was_blocked)!
68+
blocked = message.is_blocked() # ... and make sure we check for blocked after having raised the counter.
6569
message.store()
6670

6771
# Detailed log message in the following format:
68-
# TEST1234567: client=unknown[8.8.8.8], helo=myCLIENTPC, sasl_method=PLAIN, sasl_username=test@example.com, recipient_count=1, curr_count=2/1000, status=ACCEPTED
69-
log_message = 'client={}[{}], helo={}, sasl_method={}, sasl_username={}, from={}, to={}, recipient_count={}, curr_count={}/{}, status={}{}'.format(
72+
# TEST1234567: client=unknown[8.8.8.8], helo=myCLIENTPC, sasl_method=PLAIN, sasl_username=test@example.com,
73+
# recipient_count=1, curr_count=2/1000, status=ACCEPTED
74+
log_msg = 'client={}[{}], helo={}, sasl_method={}, sasl_username={}, from={}, to={}, recipient_count={}, curr_count={}/{}, status={}{}'.format( # noqa: E501
7075
message.client_name,
7176
message.client_address,
72-
request.get('helo_name'), # currently not stored in Message object or `messages` table
73-
request['sasl_method'], # currently not stored in Message object or `messages` table
77+
request.get('helo_name'), # currently not stored in Message object or `messages` table
78+
request['sasl_method'], # currently not stored in Message object or `messages` table
7479
message.sender,
7580
message.from_addr,
7681
message.to_addr,
@@ -89,11 +94,11 @@ def handle(self) -> None:
8994
self.logger.debug('Rate limit reached for %s, notifying sender via webhook!', message.sender)
9095
from .webhook import Webhook
9196
Webhook(self.conf, self.logger, message).call()
92-
self.logger.warning(log_message)
97+
self.logger.warning(log_msg)
9398
self.send_response('DEFER_IF_PERMIT ' + self.conf.get('ACTION_TEXT_BLOCKED', 'Rate limit reached, retry later'))
9499
else:
95100
self.logger.debug('Message ACCEPTED: %s', message.get_props_description())
96-
self.logger.info(log_message)
101+
self.logger.info(log_msg)
97102
self.send_response('OK')
98103

99104
def send_response(self, message: str = 'OK') -> None:
@@ -106,9 +111,9 @@ def send_response(self, message: str = 'OK') -> None:
106111

107112
def __del__(self):
108113
"""Destructor"""
109-
self.logger.msgid = None # Reset msgid in logger
114+
self.logger.msgid = None # Reset msgid in logger
110115
# TODO: Do we need to close the cursor as well? (prior to closing the connection)
111116
if self.db is not None:
112-
self.db.close() # Close database connection
117+
self.db.close() # Close database connection
113118
self.db = None
114119
self.logger.debug('Handler destroyed')

app/logging.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from .conf import Config
44
from .prefixed_logger import PrefixedLogger
55

6+
67
class Logging:
78

89
logger = None
@@ -44,7 +45,7 @@ def get_logger(conf: Config):
4445
logging.basicConfig(level=log_level, format=log_format, handlers=log_handlers)
4546
logger = logging.getLogger('policyd-rate-guard')
4647
logger.setLevel(log_level)
47-
logger.msg_prefix = conf.get('LOG_MSG_PREFIX', True) # Enable/disable custom log message prefix feature
48+
logger.msg_prefix = conf.get('LOG_MSG_PREFIX', True) # Enable/disable custom log message prefix feature
4849

4950
Logging.logger = logger
5051
return logger

app/message.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from .ratelimit import Ratelimit
22
from datetime import datetime
33

4+
45
class Message:
56

67
created_at = datetime.now()
@@ -38,7 +39,9 @@ def store(self):
3839
"""Store message in database"""
3940
self.logger.debug('Storing message')
4041
self.cursor.execute(
41-
'INSERT INTO `messages` (`ratelimit_id`, `msgid`, `sender`, `rcpt_count`, `blocked`, `from_addr`, `to_addr`, `cc_addr`, `bcc_addr`, `client_address`, `client_name`) VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)',
42+
'''INSERT INTO `messages` (`ratelimit_id`, `msgid`, `sender`, `rcpt_count`, `blocked`, `from_addr`,
43+
`to_addr`, `cc_addr`, `bcc_addr`, `client_address`, `client_name`)
44+
VALUES (%s, %s, %s, %s, %s, %s, %s, %s, %s, %s, %s)''',
4245
(
4346
self.ratelimit.get_id(),
4447
self.msgid,
@@ -59,7 +62,7 @@ def get_ratelimit(self) -> None:
5962
"""Get ratelimit for sender"""
6063
self.logger.debug('Getting ratelimit for sender {}'.format(self.sender))
6164
self.ratelimit = Ratelimit.find(self.sender, self.db, self.logger, self.conf)
62-
65+
6366
def update_ratelimit(self) -> None:
6467
"""Update ratelimit for sender"""
6568
self.logger.debug('Updating ratelimit counters for sender {}'.format(self.sender))
@@ -77,7 +80,11 @@ def is_blocked(self) -> bool:
7780
self.blocked = False
7881
return False
7982

80-
def get_props_description(self, props: list = ['sender', 'rcpt_count', 'from_addr', 'client_address', 'client_name'], separator: str = ' '):
83+
def get_props_description(
84+
self,
85+
props: list = ['sender', 'rcpt_count', 'from_addr', 'client_address', 'client_name'],
86+
separator: str = ' '
87+
) -> str:
8188
return separator.join(f"{name}={getattr(self, name)}" for name in props)
8289

8390
@staticmethod
@@ -86,7 +93,10 @@ def purge_old_messages(db_pool: object, logger: object, days: int = 90) -> None:
8693
logger.debug('Purge old messages')
8794
db = db_pool.connection()
8895
try:
89-
deleted = db.cursor().execute('DELETE FROM `messages` WHERE `created_at` < DATE_SUB(CURDATE(), INTERVAL %s DAY)', (days,))
96+
deleted = db.cursor().execute(
97+
'DELETE FROM `messages` WHERE `created_at` < DATE_SUB(CURDATE(), INTERVAL %s DAY)',
98+
(days,)
99+
)
90100
db.commit()
91101
if deleted > 0:
92102
logger.info('Deleted {} old messages (retention: {} days)'.format(deleted, days))

app/prefixed_logger.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import logging
33
from os import path
44

5+
56
class PrefixedLogger(logging.Logger):
67
"""Custom logger that prefixes messages with msgid, the filename, class and function name of the caller"""
78

@@ -28,7 +29,7 @@ def _get_caller_info(self):
2829
co_class = frame.f_locals.get('self', None).__class__.__name__ if 'self' in frame.f_locals else None
2930
co_function = frame.f_code.co_name
3031
return co_filename, co_class, co_function
31-
32+
3233
def _get_caller_frame(self):
3334
frame = inspect.currentframe().f_back
3435
try:
@@ -37,5 +38,5 @@ def _get_caller_frame(self):
3738
while frame and self.__class__.__name__ == frame.f_locals.get('self', None).__class__.__name__:
3839
frame = frame.f_back
3940
except (AttributeError):
40-
pass # Ignore AttributeError: 'NoneType' object has no attribute '__class__'
41+
pass # Ignore AttributeError: 'NoneType' object has no attribute '__class__'
4142
return frame

app/ratelimit.py

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ def __init__(
3232

3333
self.changed = self.id is None
3434

35-
3635
def store(self):
3736
"""Store ratelimit in database"""
3837
if not self.changed:
@@ -47,8 +46,9 @@ def store_new(self):
4746
"""Store new ratelimit in database"""
4847
self.logger.debug('Storing ratelimit')
4948
self.cursor.execute(
50-
"""INSERT INTO `ratelimits` (`sender`, `quota`, `quota_reset`, `quota_locked`, `msg_counter`, `rcpt_counter`, `msg_total`, `rcpt_total`)
51-
VALUES (%s, %s, %s, %s, %s, %s, %s, %s)""",
49+
'''INSERT INTO `ratelimits` (`sender`, `quota`, `quota_reset`, `quota_locked`, `msg_counter`,
50+
`rcpt_counter`, `msg_total`, `rcpt_total`)
51+
VALUES (%s, %s, %s, %s, %s, %s, %s, %s)''',
5252
(
5353
self.sender,
5454
self.quota,
@@ -68,7 +68,8 @@ def update(self):
6868
"""Update ratelimit in database"""
6969
self.logger.debug('Updating ratelimit')
7070
self.cursor.execute(
71-
'UPDATE `ratelimits` SET `quota` = %s, `msg_counter` = %s, `rcpt_counter` = %s, `msg_total` = %s, `rcpt_total` = %s WHERE `id` = %s',
71+
'''UPDATE `ratelimits` SET `quota` = %s, `msg_counter` = %s, `rcpt_counter` = %s, `msg_total` = %s,
72+
`rcpt_total` = %s WHERE `id` = %s''',
7273
(
7374
self.quota,
7475
self.msg_counter,
@@ -103,11 +104,12 @@ def add_rcpt(self, count: int):
103104
def check_over_quota(self) -> bool:
104105
"""Check if ratelimit is over quota"""
105106
self.logger.debug('Checking if ratelimit is over quota for %s', self.sender)
106-
if self.rcpt_counter > self.quota or self.msg_counter > self.quota: # rcpt_counter should always be greater than msg_counter
107+
# rcpt_counter should always be greater than msg_counter, but just in case...
108+
if self.rcpt_counter > self.quota or self.msg_counter > self.quota:
107109
self.logger.debug('Ratelimit is over quota for %s', self.sender)
108110
return True
109111
return False
110-
112+
111113
@staticmethod
112114
def find(sender: str, db: object, logger: object, conf: object) -> object:
113115
"""Get ratelimit for sender"""
@@ -136,7 +138,7 @@ def find(sender: str, db: object, logger: object, conf: object) -> object:
136138
conf,
137139
logger,
138140
)
139-
141+
140142
@staticmethod
141143
def reset_all_counters(db_pool: object, logger: object):
142144
"""Reset all ratelimit counters"""
@@ -149,7 +151,9 @@ def reset_all_counters(db_pool: object, logger: object):
149151
# reset all counters, but don't change updated_at timestamp
150152
cursor.execute('UPDATE `ratelimits` SET `msg_counter` = 0, `rcpt_counter` = 0, `updated_at` = `updated_at`')
151153
# only reset quota if it is not locked
152-
cursor.execute('UPDATE `ratelimits` SET `quota` = `quota_reset`, `updated_at` = `updated_at` WHERE `quota_locked` = 0')
154+
cursor.execute(
155+
'UPDATE `ratelimits` SET `quota` = `quota_reset`, `updated_at` = `updated_at` WHERE `quota_locked` = 0'
156+
)
153157
db.commit()
154158
finally:
155159
db.close()

app/webhook.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
11
import requests
22
from ._version import __version__ as app_version
33

4+
45
class Webhook:
56
"""Trigger webhook for sender notification"""
67

78
def __init__(self, conf: object, logger: object, message: object):
89
self.conf = conf
910
self.logger = logger
1011
self.message = message
11-
12+
1213
def call(self) -> None:
1314
"""Call webhook"""
1415
webhook_url_tpl = self.conf.get('WEBHOOK_URL')
1516
webhook_secret = self.conf.get('WEBHOOK_SECRET')
17+
if webhook_url_tpl is None or webhook_secret is None:
18+
raise ValueError('WEBHOOK_URL and WEBHOOK_SECRET must be configured')
19+
1620
user_agent = f'policyd-rate-guard/{app_version}'
1721
headers = {
1822
# 'Content-Type': 'application/json', # not needed when using 'json' param
@@ -36,7 +40,7 @@ def call(self) -> None:
3640
self.logger.info(f'Webhook call successful: POST {webhook_url_tpl} (User-Agent: {user_agent})')
3741
else:
3842
self.logger.warning(f'Webhook call failed with status code: {response.status_code} {response.text}')
39-
43+
4044
def get_data(self) -> dict:
4145
"""Get data for webhook"""
4246
return {

0 commit comments

Comments
 (0)