From 097aac5238bbac1dc2ea4f4d5bc150b265e11dc4 Mon Sep 17 00:00:00 2001 From: Amadej Kastelic Date: Mon, 15 Jul 2024 10:49:01 +0200 Subject: [PATCH] Pylint workflow --- .github/workflows/lint.yaml | 10 +++++- .pylintrc | 3 ++ bot/downloader/base.py | 6 ++-- bot/downloader/facebook/client.py | 7 ++++- bot/downloader/instagram/client.py | 2 +- bot/downloader/registry.py | 4 +-- bot/downloader/twitter/client.py | 8 ++--- bot/exceptions.py | 8 +++++ bot/integrations/discord/bot.py | 6 ++-- bot/integrations/discord/client.py | 49 ++++++++++++++++++++++-------- bot/repository.py | 4 +-- bot/service.py | 8 +++-- 12 files changed, 83 insertions(+), 32 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index fe130a9..fcaaead 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -13,7 +13,15 @@ jobs: uses: actions/setup-python@v4 with: python-version: "3.12.4" - - name: flake8 Lint + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install pipenv + pipenv install --dev + - name: flake8 uses: py-actions/flake8@v2 + - name: pylint + run: | + pipenv run pylint ./bot/**/*.py - name: black formatting uses: psf/black@stable diff --git a/.pylintrc b/.pylintrc index 75e3255..dadfe39 100644 --- a/.pylintrc +++ b/.pylintrc @@ -2,6 +2,9 @@ ignore-paths=bot/migrations +load-plugins=pylint_django +django-settings-module=examples.settings_dev + [FORMAT] max-line-length=120 diff --git a/bot/downloader/base.py b/bot/downloader/base.py index bff267d..8334bca 100644 --- a/bot/downloader/base.py +++ b/bot/downloader/base.py @@ -50,15 +50,15 @@ def to_obj(self, data, **kwargs) -> BaseClientConfig: class BaseClientSingleton: DOMAINS: typing.List[str] = [] - _INSTANCE: typing.Optional[BaseClient] = None - _CONFIG_SCHEMA: BaseClientConfigSchema = BaseClientConfigSchema + _INSTANCE: typing.Optional[typing.Union[BaseClient, int]] = None + _CONFIG_SCHEMA = BaseClientConfigSchema @classmethod def _create_instance(cls) -> None: raise NotImplementedError() @classmethod - def _load_config(cls, conf: typing.Dict) -> BaseClientConfig: + def _load_config(cls, conf: object) -> _CONFIG_SCHEMA._CONFIG_CLASS: return cls._CONFIG_SCHEMA().load(conf) @classmethod diff --git a/bot/downloader/facebook/client.py b/bot/downloader/facebook/client.py index 40ade5a..3afe708 100644 --- a/bot/downloader/facebook/client.py +++ b/bot/downloader/facebook/client.py @@ -25,6 +25,11 @@ def _create_instance(cls) -> None: cls._INSTANCE = base.MISSING return + if conf.cookies_file_path is None: + logging.warning('Not enabling facebook integration due to missing cookies') + cls._INSTANCE = base.MISSING + return + cls._INSTANCE = FacebookClient(cookies_path=conf.cookies_file_path) @@ -37,7 +42,7 @@ def __init__(self, cookies_path: str): async def get_integration_data(self, url: str) -> typing.Tuple[constants.Integration, str, typing.Optional[int]]: if url.split('?')[0].endswith('/watch') and 'v=' in url: - return self.INTEGRATION, urllib_parse.parse_qs(urllib_parse.urlparse(url).query).get('v', None), None + return self.INTEGRATION, urllib_parse.parse_qs(urllib_parse.urlparse(url).query)['v'][0], None return self.INTEGRATION, url.split('?')[0].split('/')[-1], None diff --git a/bot/downloader/instagram/client.py b/bot/downloader/instagram/client.py index 6bbc93b..37590ee 100644 --- a/bot/downloader/instagram/client.py +++ b/bot/downloader/instagram/client.py @@ -47,7 +47,7 @@ def __init__( super().__init__() self.client = instaloader.Instaloader(user_agent=user_agent) - if username and os.path.exists(session_file_path): + if username and session_file_path and os.path.exists(session_file_path): self.client.load_session_from_file(username=username, filename=session_file_path) async def get_integration_data( diff --git a/bot/downloader/registry.py b/bot/downloader/registry.py index a3eeb5d..4debf68 100644 --- a/bot/downloader/registry.py +++ b/bot/downloader/registry.py @@ -10,7 +10,7 @@ from bot.downloader.youtube import client as youtube_client -CLASSES: typing.Set[base.BaseClientSingleton] = { +CLASSES: typing.Set[typing.Type[base.BaseClientSingleton]] = { facebook_client.FacebookClientSingleton, instagram_client.InstagramClientSingleton, reddit_client.RedditClientSingleton, @@ -29,7 +29,7 @@ def should_handle(url: str) -> bool: return False -def get_instance(url: str) -> base.BaseClient: +def get_instance(url: str) -> typing.Optional[base.BaseClient]: for klass in CLASSES: if any(domain in url for domain in klass.DOMAINS): return klass.get_instance() diff --git a/bot/downloader/twitter/client.py b/bot/downloader/twitter/client.py index 0f34b58..1db5d74 100644 --- a/bot/downloader/twitter/client.py +++ b/bot/downloader/twitter/client.py @@ -61,10 +61,10 @@ def __init__( ): super().__init__() - if not all([username, email, password]): - self.client = None + self.client: typing.Optional[twscrape.API] = None + if all([username, email, password]): + self.client = twscrape.API() - self.client = twscrape.API() self.logged_in = False self.username = (username,) self.email = email @@ -86,7 +86,7 @@ async def relogin(self) -> None: async def get_post(self, url: str) -> domain.Post: uid, index = self._parse_url(url) - if not self.client: + if self.client is None: return await self._get_post_no_login(url=url, uid=uid, index=index) if not self.logged_in: diff --git a/bot/exceptions.py b/bot/exceptions.py index 14c47d5..7cf7edf 100644 --- a/bot/exceptions.py +++ b/bot/exceptions.py @@ -15,3 +15,11 @@ def __init__(self, error: str) -> None: class NotAllowedError(BaseError): def __init__(self, action: str) -> None: super().__init__(f'Action not allowed: {action}') + + +class ConfigurationError(BaseError): + pass + + +class BotError(BaseError): + pass diff --git a/bot/integrations/discord/bot.py b/bot/integrations/discord/bot.py index 8a74d70..bd3c287 100644 --- a/bot/integrations/discord/bot.py +++ b/bot/integrations/discord/bot.py @@ -1,9 +1,9 @@ -import logging import typing import discord from django.conf import settings +from bot import exceptions from bot.integrations import base from bot.integrations.discord import client @@ -22,5 +22,5 @@ def __init__(self) -> None: async def run(self) -> typing.NoReturn: if self.config.enabled and self.config.api_token: await self.client.start(token=self.config.api_token) - else: - logging.info('Discord bot not enabled or missing API token') + + raise exceptions.ConfigurationError('Discord bot not enabled or missing API token') diff --git a/bot/integrations/discord/client.py b/bot/integrations/discord/client.py index 5c95d86..8e866da 100644 --- a/bot/integrations/discord/client.py +++ b/bot/integrations/discord/client.py @@ -11,6 +11,7 @@ from bot import constants from bot import domain +from bot import exceptions from bot import service from bot.common import utils @@ -22,6 +23,10 @@ async def on_click( interaction: discord.Interaction, _: ui.Button, ) -> None: + if interaction.message is None: + logging.warning('Invoked delete on a missing message') + return + if interaction.user.mentioned_in(interaction.message): await interaction.message.delete() logging.info(f'User {interaction.user.id} performed a delete action') @@ -35,7 +40,7 @@ class DiscordClient(discord.Client): def __init__(self, *, intents: discord.Intents, **options: typing.Any) -> None: super().__init__(intents=intents, **options) - commands = [ + commands: typing.List[app_commands.Command] = [ app_commands.Command( name='embed', description='Embeds media directly into discord', @@ -67,7 +72,7 @@ async def on_ready(self): logging.info(f'Logged on as {self.user}') async def on_message(self, message: discord.Message): # noqa: C901 - if message.author == self.user: + if message.author == self.user or message.guild is None: return url = utils.find_first_url(message.content) @@ -86,6 +91,8 @@ async def on_message(self, message: discord.Message): # noqa: C901 server_uid=str(message.guild.id), author_uid=str(message.author.id), ) + if not post: + raise exceptions.IntegrationClientError('Failed to fetch post') except Exception as e: logging.error(f'Failed downloading {url}: {str(e)}') await asyncio.gather( @@ -109,6 +116,10 @@ async def on_message(self, message: discord.Message): # noqa: C901 await asyncio.gather(msg.add_reaction('❌'), new_message.delete()) async def on_reaction_add(self, reaction: discord.Reaction, user: discord.User): + if not self.user: + logging.warning('Discord bot not logged in') + return + if ( reaction.message.author.id == self.user.id and reaction.emoji == '❌' @@ -126,12 +137,14 @@ async def command_embed(self, interaction: discord.Interaction, url: str, spoile return try: - post = service.get_post( + post = await service.get_post( url=url, server_vendor=constants.ServerVendor.DISCORD, - server_uid=str(interaction.guild.id), + server_uid=str(interaction.guild_id), author_uid=str(interaction.user.id), ) + if not post: + raise exceptions.IntegrationClientError('Failed to fetch post') if not post.spoiler: post.spoiler = spoiler except Exception as e: @@ -153,7 +166,7 @@ async def command_help(self, interaction: discord.Interaction) -> None: response = service.get_server_info( server_vendor=constants.ServerVendor.DISCORD, - server_uid=str(interaction.guild.id), + server_uid=str(interaction.guild_id), ) if not response: await interaction.followup.send( @@ -168,14 +181,21 @@ async def command_help(self, interaction: discord.Interaction) -> None: async def silence_user(self, interaction: discord.Interaction, member: discord.Member, unban: bool = False) -> None: await interaction.response.defer(ephemeral=True) + if not isinstance(interaction.user, discord.Member): + await interaction.followup.send( + content='Something went wrong', + ephemeral=True, + ) + return + if interaction.user.guild.id == member.id: response = 'Can\'t ban yourself..' else: try: service.change_server_member_banned_status( server_vendor=constants.ServerVendor.DISCORD, - server_uid=interaction.guild.id, - member_uid=member.id, + server_uid=str(interaction.guild_id), + member_uid=str(member.id), banned=not unban, ) response = 'User {display_name} {banned}banned.' @@ -200,7 +220,7 @@ async def get_post_format(self, interaction: discord.Interaction, site: constant await interaction.followup.send( content=service.get_post_format( server_vendor=constants.ServerVendor.DISCORD, - server_uid=interaction.guild.id, + server_uid=str(interaction.guild_id), integration=site, ), ephemeral=True, @@ -210,7 +230,7 @@ async def _send_post( self, post: domain.Post, send_func: typing.Callable, - author: discord.User, + author: typing.Union[discord.User, discord.Member], ) -> discord.Message: send_kwargs = { 'suppress_embeds': True, @@ -238,7 +258,10 @@ async def _send_post( except discord.HTTPException as e: if e.status != 413: # Payload too large raise e - logging.info('File too large, resizing...') - file.fp.seek(0) - post.buffer = await utils.resize(buffer=file.fp, extension=extension) - return await self._send_post(post=post, send_func=send_func, author=author) + if post.buffer is not None: + logging.info('File too large, resizing...') + post.buffer.seek(0) + post.buffer = await utils.resize(buffer=post.buffer, extension=extension) + return await self._send_post(post=post, send_func=send_func, author=author) + + raise exceptions.BotError('Failed to send message') from e diff --git a/bot/repository.py b/bot/repository.py index e470d17..dd5b3ea 100644 --- a/bot/repository.py +++ b/bot/repository.py @@ -306,10 +306,10 @@ def change_server_member_banned_status( if updated == 0: models.ServerMember.objects.create( - server=models.Server.objects.filter( + server=models.Server.objects.get( vendor=server_vendor, vendor_uid=server_uid, - ).first(), + ), vendor_uid=member_uid, banned=banned, ) diff --git a/bot/service.py b/bot/service.py index f5c8ab8..d6a1410 100644 --- a/bot/service.py +++ b/bot/service.py @@ -47,7 +47,7 @@ def get_post_format( ) -async def get_post( +async def get_post( # noqa: C901 url: str, server_vendor: constants.ServerVendor, server_uid: str, @@ -61,7 +61,7 @@ async def get_post( if not client: logging.warning(f'Integration for url {url} not enabled or client init failure') - return + return None integration, integration_uid, integration_index = await client.get_integration_data(url=url) @@ -74,6 +74,10 @@ async def get_post( logging.info(f'Server {server_uid} not configured, creating a default config') server = repository.create_server(vendor=server_vendor, vendor_uid=server_uid) + if not server._internal_id: + logging.error('Internal id for server not set') + raise exceptions.BotError('Internal server error') + num_posts_in_server = repository.get_number_of_posts_in_server_from_datetime( server_id=server._internal_id, from_datetime=datetime.datetime.now() - datetime.timedelta(days=1),