diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 05009e9d..d3ae80c0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -21,10 +21,10 @@ jobs: with: access_token: ${{ github.token }} - uses: actions/checkout@v4 - - name: Set up Python 3.10 + - name: Set up Python 3.11 uses: actions/setup-python@v5 with: - python-version: '3.10' + python-version: '3.11' - uses: actions/cache@v4 with: path: ~/.cache/pip @@ -32,6 +32,8 @@ jobs: - run: pip install wheel - name: Install dependencies run: pip install -e .[dev] + - name: Run Ruff + run: ruff . - name: Run Flake8 run: flake8 - name: Black code style diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 28655ec4..22b80482 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - python-version: [3.8, 3.9, '3.10'] + python-version: [3.8, 3.9, '3.10', '3.11'] steps: - name: Cancel Outdated Runs @@ -43,7 +43,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - python-version: [3.8, 3.9, '3.10'] + python-version: [3.8, 3.9, '3.10', '3.11'] steps: - uses: actions/checkout@v4 - name: Set up Python @@ -58,9 +58,9 @@ jobs: run: pip install -e .[dev] - name: Launch test server working-directory: tests/integration_tests - run: docker-compose up -d && sleep 30 - - name: Print docker info - run: docker ps -a + run: docker compose up -d && sleep 30 + - name: Print docker services info + run: docker compose ps -a - name: Run integration tests working-directory: tests/integration_tests run: pytest . -vv -n auto diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 075d3a32..c25c201d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,7 @@ # See https://pre-commit.com/hooks.html for more hooks repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 + rev: v4.6.0 hooks: - id: end-of-file-fixer - id: fix-byte-order-marker @@ -22,16 +22,22 @@ repos: - "--filter-files" - repo: https://github.com/psf/black # Code style formatting - rev: 24.2.0 + rev: 24.8.0 hooks: - id: black exclude: (.*/)*snapshots/ - repo: https://github.com/PyCQA/flake8 # Checks the code for PEP8 violations and common pitfals - rev: 7.0.0 + rev: 7.1.1 hooks: - id: flake8 exclude: (.*/)*snapshots/ + # Ruff is a "fastar than flake8" linter/formatter + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.6.9 + hooks: + # Run the linter. + - id: ruff - repo: https://github.com/mattseymour/pre-commit-pytype rev: '2023.5.8' hooks: @@ -54,6 +60,6 @@ repos: - "88" - repo: https://github.com/pycqa/doc8 # sphinx rst style checker - rev: v1.1.1 + rev: v1.1.2 hooks: - id: doc8 diff --git a/docker-compose.yml b/compose.yaml similarity index 98% rename from docker-compose.yml rename to compose.yaml index 31cb48c6..bcb0ce4f 100644 --- a/docker-compose.yml +++ b/compose.yaml @@ -1,5 +1,3 @@ -version: "3.7" - services: app: # This docker-compose file specifies a mmpy_bot diff --git a/dev-requirements.txt b/dev-requirements.txt index 9e608516..3762129a 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,9 +1,10 @@ -black==24.2.0 +black==24.4.2 docformatter==1.7.5 -filelock==3.13.1 +filelock==3.15.4 isort==5.13.2 -flake8==7.0.0 -pytest==8.0.1 -pytest-xdist==3.5.0 -pytype==2024.2.13 +flake8==7.1.0 +pytest==8.2.2 +pytest-xdist==3.6.1 +pytype==2024.4.11 snapshottest==0.6.0 +ruff==0.1.13 \ No newline at end of file diff --git a/mmpy_bot/bot.py b/mmpy_bot/bot.py index 6a5b5f69..e9d917bf 100644 --- a/mmpy_bot/bot.py +++ b/mmpy_bot/bot.py @@ -80,7 +80,9 @@ def _register_logger(self): **{ "format": self.settings.LOG_FORMAT, "datefmt": self.settings.LOG_DATE_FORMAT, - "level": logging.DEBUG if self.settings.DEBUG else logging.INFO, + "level": ( + logging.DEBUG if self.settings.DEBUG else self.settings.LOG_LEVEL + ), "filename": self.settings.LOG_FILE, "filemode": "w", } diff --git a/mmpy_bot/driver.py b/mmpy_bot/driver.py index cb78bc17..fa5f7eec 100644 --- a/mmpy_bot/driver.py +++ b/mmpy_bot/driver.py @@ -43,7 +43,7 @@ def create_post( message: str, file_paths: Optional[Sequence[str]] = None, root_id: str = "", - props: Dict = {}, + props: Optional[Dict] = None, ephemeral_user_id: Optional[str] = None, ): """Create a post in the specified channel with the specified text. @@ -52,8 +52,8 @@ def create_post( paths are specified, those files will be uploaded to mattermost first and then attached. """ - if file_paths is None: - file_paths = [] + file_paths = file_paths or [] + props = props or {} file_ids = ( self.upload_files(file_paths, channel_id) if len(file_paths) > 0 else [] @@ -88,9 +88,9 @@ def get_post_thread(self, post_id: str): duplicate and wrongly ordered entries in the ordered list.""" thread_info = self.posts.get_post_thread(post_id) - id_stamps = [] - for id, post in thread_info["posts"].items(): - id_stamps.append((id, int(post["create_at"]))) + id_stamps = ( + (id, int(post["create_at"])) for id, post in thread_info["posts"].items() + ) # Sort the posts by their timestamps sorted_stamps = sorted(id_stamps, key=lambda x: x[-1]) # Overwrite the order with the sorted list @@ -116,7 +116,7 @@ def reply_to( message: Message, response: str, file_paths: Optional[Sequence[str]] = None, - props: Dict = {}, + props: Optional[Dict] = None, ephemeral: bool = False, direct: bool = False, ): @@ -127,8 +127,8 @@ def reply_to( Also supports replying privately by setting direct=True. """ - if file_paths is None: - file_paths = [] + file_paths = file_paths or [] + props = props or {} if direct and not message.is_direct_message: # NOTE we explicitly don't pass root_id as it would refer to a @@ -160,7 +160,7 @@ def direct_message( message: str, file_paths: Optional[Sequence[str]] = None, root_id: str = "", - props: Dict = {}, + props: Optional[Dict] = None, ephemeral_user_id: Optional[str] = None, ): # Private/direct messages are sent to a special channel that @@ -168,6 +168,7 @@ def direct_message( direct_id = self.channels.create_direct_channel([self.user_id, receiver_id])[ "id" ] + props = props or {} return self.create_post( channel_id=direct_id, @@ -199,22 +200,20 @@ def upload_files( ) -> List[str]: """Given a list of file paths and the channel id, uploads the corresponding files and returns a list their internal file IDs.""" - file_list = [] - for path in file_paths: - path = Path(path) - # Note: 'files' should be a name of an expected attribute in the body - # but seems to be ignored when simply uploading files to mattermost - file_list.append( + # Note: 'files' should be a name of an expected attribute in the body + # but seems to be ignored when simply uploading files to mattermost + file_list = [ + ( + "files", ( - "files", - ( - path.name, - Path(path).read_bytes(), - ), - ) + Path(path).name, + Path(path).read_bytes(), + ), ) + for path in file_paths + ] result = self.files.upload_file( files=file_list, data={"channel_id": channel_id} ) - return list(info["id"] for info in result["file_infos"]) + return [info["id"] for info in result["file_infos"]] diff --git a/mmpy_bot/event_handler.py b/mmpy_bot/event_handler.py index 7d70466a..e719932c 100644 --- a/mmpy_bot/event_handler.py +++ b/mmpy_bot/event_handler.py @@ -13,7 +13,7 @@ log = logging.getLogger("mmpy.event_handler") -class EventHandler(object): +class EventHandler: def __init__( self, driver: Driver, @@ -37,10 +37,8 @@ def start(self): def _should_ignore(self, message: Message): # Ignore message from senders specified in settings, and maybe from ourself return ( - True - if message.sender_name.lower() + message.sender_name.lower() in (name.lower() for name in self.settings.IGNORE_USERS) - else False ) or (self.ignore_own_messages and message.sender_name == self.driver.username) async def _check_queue_loop(self, webhook_queue: queue.Queue): @@ -75,37 +73,34 @@ async def _handle_post(self, post): # Find all the listeners that match this message, and have their plugins handle # the rest. - tasks = [] - for matcher, functions in self.plugin_manager.message_listeners.items(): - match = matcher.search(message.text) - if match: - groups = list([group for group in match.groups() if group != ""]) - for function in functions: - # Create an asyncio task to handle this callback - tasks.append( - asyncio.create_task( - function.plugin.call_function( - function, message, groups=groups - ) - ) - ) + tasks = [ + asyncio.create_task( + function.plugin.call_function( + function, + message, + groups=[group for group in match.groups() if group != ""], + ) + ) + for matcher, functions in self.plugin_manager.message_listeners.items() + if (match := matcher.search(message.text)) + for function in functions + ] + # Execute the callbacks in parallel asyncio.gather(*tasks) async def _handle_webhook(self, event: WebHookEvent): # Find all the listeners that match this webhook id, and have their plugins # handle the rest. - tasks = [] - for matcher, functions in self.plugin_manager.webhook_listeners.items(): - match = matcher.search(event.webhook_id) - if match: - for function in functions: - # Create an asyncio task to handle this callback - tasks.append( - asyncio.create_task( - function.plugin.call_function(function, event) - ) - ) + + tasks = [ + # Create an asyncio task to handle this callback + asyncio.create_task(function.plugin.call_function(function, event)) + for matcher, functions in self.plugin_manager.webhook_listeners.items() + if matcher.search(event.webhook_id) + for function in functions + ] + # If this webhook doesn't correspond to any listeners, signal the WebHookServer # to not wait for any response if len(tasks) == 0: diff --git a/mmpy_bot/function.py b/mmpy_bot/function.py index 1149d526..c738937e 100644 --- a/mmpy_bot/function.py +++ b/mmpy_bot/function.py @@ -43,7 +43,7 @@ def __init__( ) self.function = function - self.is_coroutine = asyncio.iscoroutinefunction(function) + self.is_coroutine = inspect.iscoroutinefunction(function) self.is_click_function: bool = False self.matcher = matcher self.metadata = metadata @@ -100,7 +100,7 @@ def __init__( if self.is_click_function: _function = self.function.callback - if asyncio.iscoroutinefunction(_function): + if inspect.iscoroutinefunction(_function): raise ValueError( "Combining click functions and coroutines is currently not supported!" " Consider using a regular function, which will be threaded by default." @@ -155,10 +155,10 @@ def __call__(self, message: Message, *args): assert len(args) <= 1 # There is only one group, (.*)? if len(args) == 1: # Turn space-separated string into list - args = tuple(shlex.split(args[0])) + args = shlex.split(args[0]) try: ctx = self.function.make_context( - info_name=self.plugin.__class__.__name__, args=list(args) + info_name=self.plugin.__class__.__name__, args=args ) ctx.params.update({"self": self.plugin, "message": message}) return self.function.invoke(ctx) diff --git a/mmpy_bot/settings.py b/mmpy_bot/settings.py index 7ce9aa24..628157d8 100644 --- a/mmpy_bot/settings.py +++ b/mmpy_bot/settings.py @@ -1,4 +1,5 @@ import collections +import logging import os import warnings from dataclasses import dataclass, field, fields @@ -8,9 +9,7 @@ def _get_comma_separated_list(string: str, type=str): values = string.split(",") # Convert to the specified type if necessary. - if type is not str: - values = list([type(value) for value in values]) - return values + return values if type is str else [type(value) for value in values] def _is_valid_option(_type, valid_types): @@ -59,6 +58,7 @@ class Settings: DEBUG: bool = False # Respond to channel message "!help" (without @bot) RESPOND_CHANNEL_HELP: bool = False + LOG_LEVEL: int = logging.INFO LOG_FILE: Optional[str] = None LOG_FORMAT: str = "[%(asctime)s][%(name)s][%(levelname)s] %(message)s" LOG_DATE_FORMAT: str = "%m/%d/%Y %H:%M:%S" @@ -91,6 +91,13 @@ def __post_init__(self): ) self.MATTERMOST_API_PATH = self.MATTERMOST_API_PATH[: -len(api_url)] + if self.DEBUG: + warnings.warn( + "DEBUG has been deprecated and will be removed in a future release. " + "Set LOG_LEVEL to logging.DEBUG to increase verbosity.", + DeprecationWarning, + ) + def _check_environment_variables(self): for f in fields(self): if f.name in os.environ: diff --git a/mmpy_bot/wrappers.py b/mmpy_bot/wrappers.py index 7a66bac7..58a459ee 100644 --- a/mmpy_bot/wrappers.py +++ b/mmpy_bot/wrappers.py @@ -38,6 +38,10 @@ def channel_id(self): def channel_name(self): return self.body["data"]["channel_name"] + @cached_property + def file_ids(self): + return self.body["data"]["post"].get("file_ids", []) + @cached_property def is_direct_message(self): return self.body["data"]["channel_type"] == "D" diff --git a/pyproject.toml b/pyproject.toml index 228ff322..c0aa8c8f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,3 +18,96 @@ classifiers = [ 'Topic :: Internet :: WWW/HTTP', 'Topic :: Software Development :: Libraries :: Python Modules', ] + +[tool.ruff] +ignore = [ + "A003", # Class attribute shadowing a builtin (models use "id", other interface requirements) + "B008", # Do not perform function call in arg defaults (FastAPI does this with Depends) + "D1", # Missing docstrings (all of them) + "D203", # 1 blank line required before docstring + "D206", # Use spaces, not tabs, to indent docstrings (incompatible with formatter) + "D212", # Multi-line docstring summary starts at first line + "D300", # Docstrings use triple double quotes (incompatible with formatter) + "E111", # Indentation uses 4 spaces (incompatible with formatter) + "E114", # Comment indentation uses 4 spaces (incompatible with formatter) + "E117", # Don't over-indent code (incompatible with formatter) + "ISC001", # Don't implicitly concatenate strings on a single line (imcompatible with formatter) + "ISC002", # Don't implicitly concatenate strings using backslash (imcompatible with formatter) + "PGH003", # Use specific code when ignoring typechecking (makes lines too long) + "PLW0603", # Don't update global variables within functions + "RET503", # Always use explicit return for non-None functions (some functions raise exceptions) + "RUF012", # Annotate mutable class attributes with typing.ClassVar (BaseSettings do this) + "SIM105", # Use contextlib.suppress(Exception) instead of try-except-pass (ugly) + "SIM110", # Use return any() instead of for loop (I personally find this less clear) + "W191", # Use spaces, not tabs, to indent (incompatible with formatter) +] +line-length = 100 +select = [ + # Active linter categories + "A", # flake8-builtins -- names should not shadow bulitin names (file, open, id, etc) + "B", # flake8-bugbear -- find probable bugs + "C90", # mccabe -- check complexity of functions + "D", # pydocstyle -- enforce docstring standard + "E", # pycodestyle errors -- standard lint error set + "EXE", # flake8-executable -- executable .py files should have a shebang and vice versa + "F", # pyflakes errors -- standard lint error set + "FA", # flake8-future-annotations -- check for type annotations in older Python versions + "FLY", # flynt -- check for joins when an f-string will work + "I", # isort -- sort imports + "ISC", # flake8-implicit-str-concat -- are two strings unnecessarily implicitly concatenated + "N", # pep8-naming -- check naming convention + "PERF", # perflint -- misc checks for performance-related issues + "PGH", # pygrep-hooks -- noqa must have a code included (and type:ignore, but we disable) + "PIE", # flake8-pie -- misc checks for unnecessary/useless code + "PLC", # pylint convention -- check for not using falsey string checks, etc + "PLE", # pylint error -- misc errors + "PLW", # pylint warning -- misc unsafe practices + "RET", # flake8-return -- checks around function returns + "RSE", # flake8-raise -- no unnecessary parens around raised exceptions + "RUF", # ruff -- misc checks, including unused noqa and some gotchas + "S", # flake8-bandit -- security-related checks, such as for hardcoded passwords + "SIM", # flake8-simplify -- misc checks for Python idioms being used correctly + "SLOT", # flake8-slots -- some subclasses should define __slots__ + "T10", # flake8-debugger -- check for pdb/breakpoint calls in the code + "W", # pycodestyle warnings -- standard lint warning set + + # Disabled linter categories that might be useful in the future + #"ASYNC", # flake8-async -- TODO: should probably add this and fix the issues to improve perf + #"FURB", # refurb -- use more modern Python idioms (currently preview-only) + #"LOG", # flake8-logging -- misc checks for logging idiom issues (currently preview-only) + + # Disabled linter categories that were evaluated, but that we decided not to use + #"AIR", # airflow -- we don't use Apache airflow + #"ANN", # flake8-annotations -- only if we want to require type annotations everywhere + #"ARG", # flake8-unused-arguments -- our test methods use unused args as fixtures + #"BLE", # flake8-blind-except -- only if we want to disallow "except Exception:" + #"C4", # flake8-comprehensions -- nitpicky + #"COM", # flake8-commas -- black handles this for us (most incompatible with formatter) + #"DJ", # flake8-django -- we don't use Django + #"DTZ", # flake8-datetimez -- cryptography does not use timezone-aware datetimes + #"EM", # flake8-errmsg -- nitpicky + #"ERA", # eradicate -- commented-out code is rare, and only survives a PR if useful + #"FBT", # flake8-boolean-trap -- only if we want to disallow boolean function args + #"FIX", # flake8-fixme -- we use fixme, todo, etc in our codebase + #"G", # flake8-logging-format -- only if we want to disallow "logging.error(f'{x}')" + #"ICN", # flake8-import-conventions -- nitpicky + #"INP", # flake8-no-pep420 -- incorrectly flags our migrations, unit tests, and scripts + #"INT", # flake8-gettext -- not sure what this actually does + #"NPY", # numpy -- we don't use NumPy + #"PD", # pandas-vet -- we don't use Pandas + #"PLR", # pylint refactor -- nitpicky (no magic numbers ever, functions with too many args) + #"PT", # flake8-pytest-style -- nitpicky (requires unnecessary parens for consistency, etc) + #"PTH", # flake8-use-pathlib -- nitpicky, unnecessary + #"PYI", # flake8-pyi -- only if we manually create typing stub files + #"Q", # flake8-quotes -- black handles this for us (incompatible with formatter) + #"SLF", # flake8-self -- good, but we break this rule occasionally + #"T20", # flake8-print -- nitpicky + #"TCH", # flake8-type-checking -- only if we want to avoid runtime type checking overhead + #"TD", # flake8-todos -- we don't use this process-heavy way of managing todos + #"TID", # flake8-tidy-imports -- nitpicky + #"TRY", # tryceratops -- nitpicky + #"UP", # pyupgrade -- dislikes Optional, parens around multiline strings (black conflict) + #"YTT", # flake8-2000 -- only needed if you're doing sys.version_info checks +] +src = ["mmpy_bot"] +target-version = "py311" diff --git a/requirements.txt b/requirements.txt index 63651b5b..f3f34574 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,5 @@ -aiohttp>=3.7.4.post0 -click>=7.0 -mattermostautodriver~=1.3.0 -schedule>=0.6.0 +httpx>=0.27.0,<0.28.0 +aiohttp>=3.9.5 +click>=8.1.7 +mattermostautodriver>=2.0.0 +schedule>=1.2.2 diff --git a/tests/integration_tests/docker-compose.yml b/tests/integration_tests/compose.yaml similarity index 91% rename from tests/integration_tests/docker-compose.yml rename to tests/integration_tests/compose.yaml index 111a3d88..e12285c3 100644 --- a/tests/integration_tests/docker-compose.yml +++ b/tests/integration_tests/compose.yaml @@ -1,5 +1,3 @@ -version: "3.7" - services: app: container_name: "mattermost-bot-test" diff --git a/tests/integration_tests/test_direct_message_plugin.py b/tests/integration_tests/test_direct_message_plugin.py index db73f5f6..c6e548b0 100644 --- a/tests/integration_tests/test_direct_message_plugin.py +++ b/tests/integration_tests/test_direct_message_plugin.py @@ -2,7 +2,7 @@ import time from string import ascii_letters -from .utils import start_bot # noqa, only imported so that the bot is started +from .utils import start_bot # noqa: F401, only imported so that the bot is started from .utils import MAIN_BOT_ID, OFF_TOPIC_ID, RESPONSE_TIMEOUT, TEAM_ID from .utils import driver as driver_fixture from .utils import expect_reply diff --git a/tests/integration_tests/test_example_plugin.py b/tests/integration_tests/test_example_plugin.py index 7ebf8891..e731ff9d 100644 --- a/tests/integration_tests/test_example_plugin.py +++ b/tests/integration_tests/test_example_plugin.py @@ -1,6 +1,6 @@ import time -from .utils import start_bot # noqa, only imported so that the bot is started +from .utils import start_bot # noqa: F401, only imported so that the bot is started from .utils import MAIN_BOT_ID, OFF_TOPIC_ID, RESPONSE_TIMEOUT, TEAM_ID from .utils import driver as driver_fixture from .utils import expect_reply diff --git a/tests/integration_tests/test_webhook_example.py b/tests/integration_tests/test_webhook_example.py index ebb0c4d8..338d3c34 100644 --- a/tests/integration_tests/test_webhook_example.py +++ b/tests/integration_tests/test_webhook_example.py @@ -1,7 +1,7 @@ import asyncio import time -from .utils import start_bot # noqa, only imported so that the bot is started +from .utils import start_bot # noqa: F401, only imported so that the bot is started from .utils import OFF_TOPIC_ID, RESPONSE_TIMEOUT from .utils import driver as driver_fixture from .utils import expect_reply diff --git a/tests/integration_tests/utils.py b/tests/integration_tests/utils.py index 589bce96..fe8a589c 100644 --- a/tests/integration_tests/utils.py +++ b/tests/integration_tests/utils.py @@ -34,7 +34,7 @@ def expect_reply(driver: Driver, post: Dict, wait=RESPONSE_TIMEOUT, retries=1): reply = thread_info["posts"][reply_id] break - if not reply: + if reply is None: raise ValueError("Expected a response, but didn't get any!") return reply diff --git a/tests/unit_tests/bot_test.py b/tests/unit_tests/bot_test.py index 7d2f076f..97a1d59d 100644 --- a/tests/unit_tests/bot_test.py +++ b/tests/unit_tests/bot_test.py @@ -1,3 +1,4 @@ +import logging from unittest import mock import pytest @@ -12,7 +13,7 @@ def bot(): # Patch login to avoid sending requests to the internet with mock.patch("mmpy_bot.driver.Driver.login") as login: - bot = Bot(plugins=[ExamplePlugin()], settings=Settings(DEBUG=True)) + bot = Bot(plugins=[ExamplePlugin()], settings=Settings(LOG_LEVEL=logging.DEBUG)) login.assert_called_once() yield bot bot.stop() # if the bot was started, stop it