Spunky Sputniks #9
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This project contains documentation and recommended tooling configuration for projects to be submitted during a Python Discord Code Jam. Co-authored-by: Akarys42 <[email protected]> Co-authored-by: ChrisLovering <[email protected]> Co-authored-by: mbaruh <[email protected]> Co-authored-by: fisher60 <[email protected]>
Changes the local pre-commit hooks out for remote ones, as well as changes the action to use the pre-commit action, so pre-commit will no longer depend on your project manager's settings.
Global Actions
Renames workflow to Lint
Signed-off-by: GitHub <[email protected]>
Signed-off-by: GitHub <[email protected]>
Signed-off-by: GitHub <[email protected]>
Signed-off-by: GitHub <[email protected]>
Signed-off-by: GitHub <[email protected]>
- Use Python 3.10 instead of 3.9 - Update action versions to the newest major versions - Remove caching configuration since it's not doing anything useful (and we would rather not have teams deal with caching issues)
Signed-off-by: GitHub <[email protected]>
*although I added '.0' at the end of flake8-docstrings's requirement since it's present in the bot repository (where Kat got her versions anyway).
Update packaging and CI config for CJ9
Unfortunately installing isort from source errors out due to a poetry bug(?). Since isort 5.x has a stability policy active, let's just bump this so the template is usable (and upgrade the template properly later).
The screenshot is my own as I couldn't find a GitHub provided one that works unfortunately. Admittedly ugly.
Co-authored-by: Gimpy3887 <[email protected]> Co-authored-by: Peter Bierma <[email protected]>
Demobot is complete
Co-authored-by: enskyeing <[email protected]>
Co-authored-by: enskyeing <[email protected]>
mbaruh
reviewed
Sep 1, 2024
There was a problem hiding this comment.
It's clear a lot of effort went into this project, well done. You wrote a library, gave it proper documentation, and prepared a demo all in the span of the code jam.
It's clear some things had to be rushed for it to be ready in time, but all the same I wrote comments below for your consideration. Hopefully you enjoyed building the project :)
| description = "Database library using nothing but Discord. PyDis Codejam 2024." | ||
| readme = "README.md" | ||
| license = "MIT" | ||
| dependencies = ["discord.py", "pydantic", "typing_extensions", "loguru", "aiocache"] |
There was a problem hiding this comment.
Should lock the dependencies to specific versions like you did in hatch.toml
Comment on lines
+25
to
+26
| except Exception as e: | ||
| print(f"{e.__class__.__name__}: {e}") |
There was a problem hiding this comment.
I have a few issues with this:
- It's better to print the full traceback in such cases, rather than just the error message.
- discord.py states which exceptions can be raised from
tree.sync, so catching a base Exception is not justified here. - Why not
logger.exception? - If syncing fails, your bot is left in an odd state that isn't handled here.
Comment on lines
+28
to
+29
| async def on_error(self, event_method: str, /, *args: asyncio.Any, **kwargs: asyncio.Any) -> None: | ||
| return await super().on_error(event_method, *args, **kwargs) |
There was a problem hiding this comment.
What does overriding the method do here, if you're just calling the super method as-is?
Comment on lines
+35
to
+39
| async with db.conn(os.getenv("DB_BOT_TOKEN")): | ||
| try: | ||
| await bot.start(os.getenv("BOOKMARK_BOT_TOKEN")) | ||
| finally: | ||
| await bot.close() |
Comment on lines
+34
to
+47
| def build_bookmark_embed(record: models.BookmarkedMessage): | ||
| embed = discord.Embed(title=record.title, description=record.message_content, colour=0x68C290) | ||
| embed.set_author( | ||
| name=record.author_name, | ||
| icon_url=record.author_avatar_url | ||
| ) | ||
| return embed | ||
|
|
||
| def build_embeds_list(records: list[models.BookmarkedMessage]) -> list[discord.Embed]: | ||
| embeds: list[discord.Embed] = [] | ||
| for record in records: | ||
| embed = build_bookmark_embed(record) | ||
| embeds.append(embed) | ||
| return embeds |
There was a problem hiding this comment.
I won't mark it everywhere, but you have several places with weird indentation.
| self, | ||
| interaction: discord.Interaction, | ||
| table: discord.TextChannel, | ||
| record: str, |
There was a problem hiding this comment.
It would be nicer if you could specify a record ID instead.
| await interaction.edit_original_response( | ||
| content=f"Table `{table_name}` found! Gathering data..." | ||
| ) | ||
| except IndexError as e: |
Comment on lines
+28
to
+34
| await interaction.response.send_message( | ||
| content=f"Searching for table `{name}`..." | ||
| ) | ||
| table_name = name.name.replace("-", " ").lower() | ||
|
|
||
| try: | ||
| table = self.db.tables[table_name] |
There was a problem hiding this comment.
A table resolution helper function would be useful here as well. A lot of code that repeats itself with small variations.
Comment on lines
+60
to
+62
| for game in table_values: | ||
| for col in table_columns: | ||
| data[col].append(getattr(game, col)) |
Comment on lines
+185
to
+188
| table_info: list | None = None | ||
| table_schema: dict | None = None | ||
| schemas: list[dict] | None = None | ||
| embed_gen: discord.Embed | None = None |
There was a problem hiding this comment.
"declaring" these ahead of time isn't necessary
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.