#dev-log

1 messages ยท Page 2 of 1

regal archBOT
#

We'd probably only implement taking stdin in snekbox if we wanted to have it for the bot, and I'm not convinced it's worth the effort given it's usually pretty simple to just replace the input with a fixed string.

I think that's a separate issue that could be opened on this repo though. Even if we did support input users would get an EOFError if they forgot to supply it, so improving that response would make sense anyway.

#
[python-discord/site] New branch created: extend\-download\-timeout
odd spireBOT
regal archBOT
#
[python-discord/bot-core] branch deleted: invite\-regex
regal archBOT
regal archBOT
regal archBOT
regal archBOT
#

Would it be best to have this implemented on a keyword basis? Meaning that we have a list of particular keywords that map to a particular rule?
Or would it be better to use NLP here in order to try to identify the rule that a particular user is trying to mention, and then maybe give suggestions ?

I think having keywords would be best, it should be simpler, as well as being more predictable and giving us more control.

With that we would need some way of discovering the keywords that work...

#

IMO regardless of what we do, we should keep the ability to reference based on numbers, on top of whatever we do here.

The rule command has existed for quite a while that a lot of staff and regulars are very used to be able to invoke based on an index.

In terms of implementation, this is trivial, as we would just need to leave the greedy int convertor, and add an optional string param.

#

@ChrisLovering I don't think the point was to change the fundamentals of how the command works. Listing a rule by its number would remain the same.
What we're trying to do is to make it slightly easier for people who are unacquainted with the rule numbers to find them easily.

@wookie184 I definitely agree with you on that. Because we don't want it to get too clunky and get lost in finding keywords as well.

I'll try to spin up a draft PR for this and discuss implementation details there...

regal archBOT
#

How much of an issue would it be to take a keyword and check whether it appears only in a particular rule? In terms of getting undesirable results

Not so much undesirable results, but it would stop us having stuff like !rule tos if the rule just contains "terms of service", or !rule advert if the rule contains advertisment.

The rules are fetched from the API

It shouldn't be too difficult to add the keywords on the site side and make them be fetched along with the rules.

regal archBOT
#

One possibility would be trying to work out the syntax highlighting based on whatever is being used on the actual doc page. Taking a quick look I can't see a way that woud actually be done, that information seems to be lost by the time the doc page is actually rendered.

Another idea would be to try parsing the contents of the codeblock, and see if it parses as valid python, like how we do already to detect people sending code without codeblocks https://github.com/python-discord/bot/blob/ma...

regal archBOT
#

Sentry Issue: BOT-366

HTTPException: 400 Bad Request (error code: 50035): Invalid Form Body
In embeds.0.title: Must be 256 or fewer in length.
(1 additional frame(s) were not displayed)
...
  File "discord/ext/commands/help.py", line 960, in command_callback
    return await self.send_error_message(string)
  File "bot/exts/info/help.py", line 273, in send_error_message
    await self.context....
#

I think having control over the config directly is quite useful.

So maybe instead we could have an additional optional param for difficulty. This param would set the "default" config for the game, but users could still give the existing params to overwrite that if needed.

IE you could do !hangman easy 4 which would set the min_length to 4 and the other variables to whatever would be considered "easy".

What do you think of that?

#

Sentry Issue: BOT-379

Forbidden: 403 Forbidden (error code: 30010): Maximum number of reactions reached (20)
  File "discord/client.py", line 456, in _run_event
    await coro(*args, **kwargs)
  File "bot/exts/fun/duck_pond.py", line 189, in on_raw_reaction_add
    await self.locked_relay(message)
  File "bot/exts/fun/duck_pond.py", line 120, in locked_relay
    await message.add_reaction("โœ…"...
regal archBOT
#

For the implementation of the button, I would ideally add the multiple arguments to the init of the view class and call the button with the arguments how it's being called currently, right?

Yes, I think that sounds right. There are some examples of discord.py views with buttons here if that helps: https://github.com/python-discord/sir-robin/blob/e90d24072baf2dc57acd455e8a9d7e5d40457d46/bot/exts/code_jams/_views.py

#

limit of 1 new thread per day across all the topical channels per person

I'm not sure about this, especially if the only way to enforce it would be auto-deleting further threads, which would be quite annoying (not sure we could easily prevent opening threads with permissions?).

If someone was helping they could reasonably want to reply in threads to someones message in a fast-moving channel, so I think there's a pretty good use case for opening lots of threads in a day, especially in ...

regal archBOT
regal archBOT
#

@wookie184
I was thinking of how this would be implemented:

  1. Do we keep this as 1 command only or do we have a group of commands? The "main" one would be using the rule number like usual & the other with a keyword.
  2. If we decide to have one command, I suppose that we need to prioritize numbers on keywords, this leads to having a signature like this: async def rules(self, ctx: Context, rules: Greedy[int], keyword: Optional[str]) -> None
    --> If no number is provided, we will try to...
regal archBOT
#

It would be clearer to just have a loop that does all the checks necessary before incrementing the total. The logic to get the replied to message is also quite involved, so could be moved to a separate function, possibly in bot.utils.messages. You'd then have something more like this:
...

Yeah, I completely agree. The current logic was done by @onerandomusername and I didn't think to adjust it. I'll do this :+1:

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

872281e Added mod alerted notice to auto-infractions - ionite34
e68868e Added newlines and non-mentioned mod role - ionite34
c9d5aac Added article to automute message - ionite34
a4a7c64 Merge branch 'main' into automute-message - minalike
a441f9d Merge pull request #2251 from python-discord/au... - minalike

#
[python-discord/bot] branch deleted: automute\-message
oak estuaryBOT
odd spireBOT
regal archBOT
#

Closes issue #1565.

Due to the nature of the bot internals, it would be messy to only change the unban command to take a reason argument. Therefore, all pardon commands that call the method pardon_infraction() provided by the InfractionScheduler cog will have a reason argument and append the pardon reason to the infraction through InfractionScheduler.deactivate_infraction()'s HTTP patch call to the moderation API when the infraction is marked as non-active.

If anyone would like to...

regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
#

Do we keep this as 1 command only or do we have a group of commands?

It should remain as one command, here are examples of how I think it should work:
!rule -> Shows the general rule message as it did before.
!rule 1 -> Shows rule 1 as it did before
!rule 1 2 -> Shows rule 1 and 2 as it did before.
!rule coc -> Shows the rule with keyword "coc".
!rule 1 Look at this rule -> Only shows rule 1, i.e. ignore the text if a number was already given.
!rule a b c -> There are a...

regal archBOT
#
        remaining = time.humanize_delta(expiry, arrow.utcnow(), max_units=2)
        if duration != remaining:
            duration += f" ({remaining} remaining)"

This is causing an issue to still happen sometimes for me. Since it's comparing to arrow.utcnow() if the current time has changed to the next second between infraction creation and this line there will be a difference.

I believe the intention of this was to make it clear that the duration is not from when...

#

fce6ea2 feat: command for banning compromised accounts - dawnofmidnight
a7234a8 fix: change use of arrow to datetime and change... - dawnofmidnight
4eaecb3 fix: replace datetime.utcnow() with arrow - dawnofmidnight
2ea1d96 Merge branch 'main' into dawnofmidnight/compban - wookie184
65216f8 Merge pull request #2250 from python-discord/da... - wookie184

#
[python-discord/bot] branch deleted: dawnofmidnight/compban
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
#
[python-discord/bot] branch deleted: incident\-archive\-msg\-improvements
odd spireBOT
regal archBOT
oak estuaryBOT
odd spireBOT
regal archBOT
#
[python-discord/bot-core] New branch created: dpy\-2\.0
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/sir-lancebot] branch deleted: help\_command\_fix\_863
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
[python-discord/modmail] branch deleted: add\-git\-to\-docker\-image
#
[python-discord/modmail] branch deleted: dep\-update
#
[python-discord/modmail] New branch created: development
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
#
[python-discord/modmail] New branch created: deployment
#
[python-discord/site] branch deleted: fix\-infraction\-tests
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#

While working on upgrading some of our dependencies, I started running into issues due to us continuing to use deprecated features. This includes:

  • django-filter. The history behind django-filter, and our usage off it is a little complicated. I tried to summarize in 6fe491c, but I can elaborate/explain further if needed. The warnings go from deprecations to removal in the current latest version, which is where I ran into issues.
  • Django 5.x: django actually emits a helpful deprecation war...
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/site] New branch created: bump\-dependencies
odd spireBOT
regal archBOT
#

Blocked by #768, which this branch is based on.

This PR updates the python version to 3.10, as we've done elsewhere in our repositories, and bumps and locks all dependencies to latest versions, using exact specifiers. Using exact specifiers is part of a larger push by the dev-core team for better dependency management, along with the addition of dependabot.

To keep things as simple as possible to review, the commits are organized as follows:

  • 8210299 updates all normal dependencies, w...
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
#

This lookup fails for any non-cached message for me. Looking at the source code for the converter, after a cache-miss, it'll try to get the channel, then fetch the message from it, but it always fails to get the channel, even when it has already done other operations in that channel (such as sending a bookmark or a failure).

The reason I commented on this portion specifically is that I think it's best to just not mention this, and not worry about it.

odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#
[python-discord/sir-lancebot] branch deleted: bookmark\-enhancements
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
regal archBOT
#
[python-discord/bot] branch deleted: infraction\-durations
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
regal archBOT
#
[python-discord/sir-lancebot] New branch created: command\-match\-errors
regal archBOT
#

Relevant Issues

https://github.com/python-discord/sir-lancebot/issues/932

<!-- Link the issue by typing: "Closes #" (Closes #0 to close issue 0 for example). -->
Closes #932

Description

I replaced the reactions used currently with buttons

Did you:

  • [x] Join the Python Discord Community?
  • [x] Read all the comments in this template?
  • [x] Ensure there is an issue open, or link relevant discord discussions?
  • [x] Read and agre...
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#

I'm not sure how well having mentions in embeds will work, given they often don't resolve. Would it be better to make this a normal text message with allowed_mentions to prevent mentioning users. The character limit is much more limiting unless we use 3 separate messages though.

I think I should also add some sort of overflow handling if there are too many users. I'm thinking I might shuffle each tier and pick the first 10 or something.

regal archBOT
regal archBOT
regal archBOT
regal archBOT
#

Hello, This code returns a message.content = 0/empty
Can you point me in the right direction?

client = discord.Client(intents=discord.Intents.default())

@client.event
async def on_ready():
print('We have a logged as {0.user}'.format(client))

quote='test response'

@client.event
async def on_message(message):
print(message)
print(len(message.content))
print('Content' + str(message.content))
print(message.author)
if message.author == client.user:
...

regal archBOT
#

It looks like we have a full on discord.ui.View subclass for a simple link button - but for the actual button that sends the bookmark in the DM, we're just doing:

view = discord.ui.View(timeout=TIMEOUT)
view.add_item(
    SendBookmark(self.action_bookmark, ctx.author, ctx.channel, target_message)
)

I think this should be flipped around. There's no need for a full fledged subclass for a simple link button, but maybe we can use a view subclass for the actual bookmark. I doubt...

regal archBOT
#

It looks like we have a full on discord.ui.View subclass for a simple link button - but for the actual button that sends the bookmark in the DM, we're just doing:

view = discord.ui.View(timeout=TIMEOUT)
view.add_item(
    SendBookmark(self.action_bookmark, ctx.author, ctx.channel, target_message)
)

I think this should be flipped around. There's no need for a full fledged subclass for a simple link button, but maybe we can use a view subclass for ...

regal archBOT
regal archBOT
regal archBOT
regal archBOT
#

I have been looking into this and have a few questions. I have currently implemented a dark theme for the pre-defined colors in the BULMA_SETTINGS, the colors only change for the nav bar i.e colors present in the BULMA_SETTINGS are only used in the css of nav bar. To have a full dark theme I think we need to have background color too, will the bsckground color be implemented at (base.css)[https://github.com/python-discord/site/blob/main/pydis_site/static/css/base/base.css#L12]? base.css i...

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

Oops, I should have linked my draft PR on here so people could use it for reference. #657 - site preview
It is now a stale PR and can be ignored.
@Ibrahim2750mi, if you wish to take over I can reassign this issue to you.

In regards to your question about the background colour, from when I was creating this draft I am pretty sure I used the class has-background-white-bis to set let Bulma hook into the background-color css attr...

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#
[python-discord/king-arthur] New branch created: bot\-core\-integration
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
#
[python-discord/king-arthur] branch deleted: bot\-core\-integration
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/metricity] New branch created: stable\-d\.py\-and\-python\-3\.10
#
[python-discord/king-arthur] New branch created: botocore\-monkeypatching
odd spireBOT
regal archBOT
#
jb3
[python-discord/workers] New branch created: joe/wrangler\-upgrades
#
jb3

This PR upgrades all workers to be compatible with Wrangler 2, we still use the service-worker format for now but all wrangler.toml files for our JavaScript/TypeScript workers now work fine.

There is still a TODO to give this treatment to the Rust worker for serving robots.txt (or alternatively, ditching it and doing it in Django ๐Ÿ˜ƒ)

All dependency files have been re-locked as well.

regal archBOT
#
[python-discord/king-arthur] branch deleted: botocore\-monkeypatching
odd spireBOT
regal archBOT
regal archBOT
#

Hmm, that dash is for tags, not tag groups, so I think it's still necessary (for instance args-kwargs). I hadn't considered tag groups at all when writing this, but I don't think they're separated by spaces either, I think the tag code does not support that. The relevant code for parsing tag commands is:

https://github.com/python-discord/bot/blob/dc9cb7910040f32d91aa546eec23b942a5a72203/bot/exts/info/tags.py#L66-L68

which leads me to believe that tags currently have to follow the `(...

odd spireBOT
odd spireBOT
regal archBOT
#

Actually it might be pretty difficult to accurately discern what's a tag group, and what's a tag with a normal sentence after it (for instance this tag link at the bottom of args-kwargs which has a sentence after it). The only reliable way would be to either make a request to the page (reverse does not validate kwargs), or to use the get_tag function, but that can make an extra request (commit data) if the tag is ...

regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/king-arthur] New branch created: bump\-dep
odd spireBOT
regal archBOT
#
[python-discord/king-arthur] New branch created: bump\-dep
odd spireBOT
regal archBOT
#
[python-discord/modmail] New branch created: patch\-message\-content\-intent
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
#

Description

This bumps the bot up to 3.10, bumps all deps. The bumped deps include bot-core which comes with a move to the stable Discord.py 2.0 release.

Sir-Lancebot never had the async cog loading treatment, so this PR also includes that.

There are also miscellaneous fixes included in here for breaking changes in dep bumps.

Did you:

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/modmail] branch deleted: patch\-message\-content\-intent
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
regal archBOT
regal archBOT
#

After gathering some internal feedback, we feel the usage of the topic command outside of python-general, off-topic, and occasionally discord-bots is not high enough to require additional topics. While there is no harm in adding more, the current suggestions aren't compelling enough to add to the current list.

We do have interest however, in boosting the current topic questions for our more traffic-heavy channels where the command is invoked frequently making the existing topics very repetit...

regal archBOT
#

Description

  • Some dates are mm/dd/yyyy and others are dd/mm/yyyy. Make this consistent one way or another. Perhaps use localized timestamps since we have that option now instead of using the embed footers.
  • Nitpick: I think "minalike's GitHub profile info" should instead read: "minalike's GitHub Profile" (no backticks, proper casing)
  • Nitpick: The usage of a large thumbnail in the user embed versus a small icon for the repo embed is weird. I think I prefer the small icon.

...

regal archBOT
oak estuaryBOT
#

Doc item doc_item.symbol_id='disnake.APIMessageCommand.nsfw' present in loaded documentation inventories not found on site, inventories may need to be refreshed.

regal archBOT
#

The current invite regex is too permissive. It captures any non-space character, while invite codes can only have letters and numbers.

https://github.com/python-discord/bot-core/blob/127d0efa219d29e15bd71339d0d2517945aed57c/botcore/utils/regex.py#L15

In https://github.com/python-discord/bot-core/commit/c9daa6f3774380794968cf28faf9ba4bde1f68e2 we made it go until the whitespace because of the way the Discord client parses invites, but we can safely account for it by adding / to the cha...

regal archBOT
#
[python-discord/sir-lancebot] New branch created: message\-content\-intent
#

Description

1<<15 is the flag for message content intent, so adding this value to the intents value hard codes that to always be enabled.

Doing it this way also bypasses the need to patch the flags themselvse in discord.py.

This is required until https://github.com/python-discord/sir-lancebot/pull/1092 is merged.

Did you:

odd spireBOT
regal archBOT
#
[python-discord/sir-lancebot] branch deleted: message\-content\-intent
odd spireBOT
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
odd spireBOT
regal archBOT
#

Relevant Issues

<!-- Link the issue by typing: "Closes #" (Closes #0 to close issue 0 for example). -->
Closes #932

Description

I used buttons for the bookmark command, both to view the bookmarked message from DM's and for other members to bookmark a message that someone else invoked the command on.

Did you:

  • [x] Join the Python Discord Community?
  • [x] Read all the comments in this template?
  • [x] Ensure there is an issue ope...
oak estuaryBOT
#

Doc item doc_item.symbol_id='nextcord.Embed.Empty' present in loaded documentation inventories not found on site, inventories may need to be refreshed.

odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
#
[python-discord/sir-robin] New branch created: bump\-dpy\-botbase
regal archBOT
#

The current alert embed for a triggered filter puts the filter type in the main body, where it's easily missed - especially for those of us who have higher-res screens.

Having the triggering filter in the embed's title will give instant access to that information at a glance. Below image is a mockup of what I propose:

I am not precious about the exact formatting of the title, as long as the information is in there.

odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
thin oysterBOT
#
Sir Robin

Connected!

regal archBOT
#

Given the actual time this is run could differ by up to 18 hours each week, I think it would be possible that any autobans added in (up to) the first 18 hours after a message may not appear in the next message.

We could make this -8 days (or -7 and 18 hours), if we'd rather have repeats than missed ones. I don't mind either way. I don't think it's worth a more complex solution to 'fix' this though, as it seems pretty minor.

regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

We designed the verification system before we had components and interactions. Discord has changed quite a bit since, and we should revisit the current setup.

Current State

Users have to run a specific command, and if they ran it correctly they'll get a response. The invocation and response disappear after a bit, and if they didn't run it correctly their message disappears almost immediately.

Proposal

This setup can be converted to a button that is added to the explanation mess...

odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/sir-lancebot] New branch created: fix\-issue\-1090
odd spireBOT
regal archBOT
regal archBOT
#

Relevant Issues

<!-- Link the issue by typing: "Closes #" (Closes #0 to close issue 0 for example). -->
Closes #1090.

Description

Removed the call to self.on_command_error, so that errors from command suggestions are ignored. Also fixed an incorrect typehint that was added in #1064.

Did you:

  • [x] Join the Python Discord Community?
  • [x] Read all the comments in this template?
  • [x] Ensure there is an issue open, or link re...
odd spireBOT
regal archBOT
#

Because the command currently supports multiple rule integers, I believe it would be intuitive to also support multiple keywords. Ideally, something like !rules 1 keyword 2 keyword2 would work. However, the current command allows invocations like !rules 1 2 3 some text here, and it ignores the "some text here". I'm not sure if this could still be supported given my above proposal.

Need to gather more opinions on what the interface should be.

#
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
night lilyBOT
#
Sir Lancebot

Connected!

regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
#
[python-discord/bot] New branch created: fix\-ensure\-cached\-claimant\-none\-check
#

Minor bug I noticed locally, embed description can be None so we needed to check for it to avoid this error:

bot_1          | Traceback (most recent call last):
bot_1          |   File "/usr/local/lib/python3.10/site-packages/discord/ext/commands/bot.py", line 929, in _load_from_module_spec
bot_1          |     await setup(self)
bot_1          |   File "/bot/bot/exts/help_channels/__init__.py", line 40, in setup
bot_1          |     await bot.add_cog(HelpChannels(bot))
bot_1     ...
odd spireBOT
regal archBOT
regal archBOT
#

Users should not need to check that self.stats is not None every time they try to use it, as it never should be None.

We can avoid this by internally using a self._stats attribute that can be None, and exposing a stats property that raises an error if self._stats is None, and otherwise returns the stats client.

I think we should also do this for self.api_client. Whilst it is a slightly different case in that it can be None, it should either always be None or *nev...

#

I think all the changes so far are improvements, I definitely think not being in italics makes reading longer text cleaner and easier.

RE the two that you couldn't trigger, I think they could be left as is and this changed to be italics:
https://github.com/python-discord/bot/blob/dc9cb7910040f32d91aa546eec23b942a5a72203/bot/pagination.py#L239
My reasoning being that these are all somewhat "meta", I also wouldn't mind just making none of them in italics. I'd like to get another opinion fi...

regal archBOT
#

I think the first one (Account created at) is a localised embed timestamp (shows dd/mm/yyyy for me), whereas the second one isn't, as localized timestamps in embed footers are very limited. I don't really have a preference on how to fix this inconsistency.

I agree with your second point on changing the title wording/formatting.

I don't really mind the icons being different. I quite like the bigger icon on the user command.

regal archBOT
regal archBOT
#
[python-discord/bot] New branch created: fix\-silence\-and\-tags\-when\-unloaded
#

Previously, if the commands were not loaded and you attempted to invoke a command that doesn't exist (e.g. !hello), an error would be raised:

bot_1          | Traceback (most recent call last):
bot_1          |   File "/usr/local/lib/python3.10/site-packages/discord/client.py", line 456, in _run_event
bot_1          |     await coro(*args, **kwargs)
bot_1          |   File "/bot/bot/exts/backend/error_handler.py", line 67, in on_command_error
bot_1          |     if await self.try...
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

I've only tested it quickly so this is mainly a code review (although it seems to work nicely, I like the embeds!)

Firstly, thanks for the PR! The structure of the code seems good and it's been written well.

My main comments here are about repetition, there are couple of places where I think using loops would neaten the code up a bit, making it a bit more concise and extensible. That said, these aren't required changes, i've only mentioned them as something to think about if you want to.

odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/bot-core] New branch created: poetry\-1\.2\.0
odd spireBOT
regal archBOT
#
[python-discord/king-arthur] New branch created: poetry\-1\.2\.0
odd spireBOT
regal archBOT
#
[python-discord/sir-robin] branch deleted: bump\-dpy\-botbase
thin oysterBOT
#
Sir Robin

Connected!

regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/king-arthur] branch deleted: poetry\-1\.2\.0
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

This has an inherent dependency on how the patreon bot works. If the patreon bot decided to take away and giveth the new role in two seperate api requests, this would cease to work. Or, if the patreon bot were to take the old role away and then give the new role, this would also cease to work.

This all hinges on the patreon bot making a single api request, and would lead to funky behaviour if their bot used more than one request--and I believe it does.

regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#

I don't think this should be an issue. Whatever order things happen there will still only be one member_update that involves the tier increasing, which is all that this checks.

The only slightly unintended case would be if a user moves down a tier (e.g. from 3 to 2), and the bot does this by removing 3 and then adding 2. In this case the message would be sent, although it still wouldn't be incorrect (Just saying they are now a tier 2 patron).

Fixing this would probably require waiting f...

regal archBOT
odd spireBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#

Before the Code Jam some features (the majority of them) weren't implemented in an optimal manner, hence a facelift PR is required.
A few notes from previous PRs:

  • There is a common pattern in Sir Robin where we get a team, handle possible API errors, and the proceed with the data gathered from the API. This procedure should be moved to a helper function.
    • Return the data as dataclasses (/pydantic models), so that we don't have to slice dicts everywhere.
  • Set the Textbox's type to b...
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/bot] branch deleted: pin\-poetry\-to\-1\.1\.X
#

350df44 Use venvs with poetry in Dockerfile - ChrisLovering
e12e591 Use HassanAbouelela/setup-python for CI - ChrisLovering
0b495e7 Bump poetry in Docker and lint to 1.2.0 - ChrisLovering
03a3a03 Don't use fake in-project venvs for poetry - ChrisLovering
7adc758 Merge pull request #2268 from python-discord/pi... - HassanAbouelela

odd spireBOT
oak estuaryBOT
regal archBOT
#
[python-discord/bot] New branch created: ignore\-mounted\-project\-venvs
#

Poetry's virtualenvs.in-project config deafults to None, meaning it will use in-project venvs if it finds one, otherwise it will use the cache dir. In dev we mount the entire root project directory to /bot. This means if the host's venv in in the project dir, this will get mounted and prioritised by poetry run. If the host is on a non-linux OS this will cause poetry to fail to boot.

Specify the path for poetry venvs
Without this the venv would be created in /root/.cache and the nonn-root...

odd spireBOT
oak estuaryBOT
regal archBOT
#

We wouldn't want, for example, for someone to eval a message from 10 hours ago, especially in help channels

You're right, we wouldn't. We could probably add some config option for this in config.yml or similar, but I think 2 minutes seems reasonable. As for the message popping up without any context, not necessarily. You can see which user ran the command, and you can click on it to be taken back to the original message that eval was ran on.

Since the input message might not have ...

regal archBOT
regal archBOT
regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/bot-core] New branch created: check\-rerun\-job\-util
#

Uses the logic of the eval command in python-discord/bot to create a re-run util.

That is, if the passed context's message content is edited within REDO_TIMEOUT seconds, and the user then reacts with the ๐Ÿ” emoji that appears, then the util will return the new content, indicating that the job should be re-run. Otherwise, None is returned, indicating the job shouldn't be re-run.

This is made since python-discord/sir-lancebot#1062 asks for re-ru...

oak estuaryBOT
#

Doc item doc_item.symbol_id='disnake.WidgetMember.bot' present in loaded documentation inventories not found on site, inventories may need to be refreshed.

regal archBOT
#

Description

Hello, the feature I would like to propose is a cellular automaton generator.
like if someone types;
.simulate
and it would send a picture of the output.

Reasoning

Proposed Implementation

the code for the simulation is already implemented in one of my own bots and it's fully functional
(https://github.com/Sas2k/PokeStrike-Utility-Bot/blob/main/Cogs/Fun.py#L19-L72)[Link-To-Gitub-Repo-of-the-existing-code]

![image](https://user-images.githubusercontent....

regal archBOT
regal archBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
#
jb3

71b1802 Ignore mounted in-project venvs on startup - ChrisLovering
2db6a24 Specify the path for poetry venvs - ChrisLovering
9ee334d Merge branch 'main' into ignore-mounted-project... - jb3
5b46ae8 Merge pull request #2276 from python-discord/ig... - jb3

#
jb3
[python-discord/bot] branch deleted: ignore\-mounted\-project\-venvs
odd spireBOT
oak estuaryBOT
odd spireBOT
odd spireBOT
regal archBOT
#

There are a couple of cases that have come up that I didn't consider in my original proposed specification https://github.com/python-discord/bot/issues/2108#issuecomment-1219378065. Here are some updated cases, and how I think they should work (all still open to discussion).

!rule 1 coc 2 - Show rule 1 and 2 (coc is rule 1, shouldn't repeat it)
!rule 1 hello 2 coc - Show rule 1 (only)
!rule 999 coc - Error about invalid indices
!rule coc 999 - Error about invalid indices
`!rule...

odd spireBOT
regal archBOT
#

40ab60f Allow referencing message as argument to !remi... - TizzySaurus [29d8820](https://github.com/python-discord/bot/commit/29d882065e12f9a042b14d6dfaf3161af215c953) Make reference message in reminders italic. - TizzySaurus [2536152](https://github.com/python-discord/bot/commit/2536152ffe43a9058dffc03fd8939015c0fd3d37) Merge branch 'main' into bot-2231-bug - TizzySaurus [9c85342](https://github.com/python-discord/bot/commit/9c85342f4449f2116fb36ee1500ad06a94c1e14c) Update docstrings & comment. - TizzySaurus [15e0491`](https://github.com/python-discord/bot/commit/15e0491a3ba533a2423d44b415de355d1152f84c) Merge remote-tracking branch 'origin/bot-2231-b... - TizzySaurus

#
[python-discord/bot] branch deleted: bot\-2231\-bug
odd spireBOT
oak estuaryBOT
odd spireBOT
odd spireBOT
regal archBOT
#

There are a couple of cases that have come up that I didn't consider in my original proposed specification #2108 (comment). Here are some updated cases, and how I think they should work (all still open to discussion).

!rule 1 coc 2 - Show rule 1 and 2 (coc is rule 1, shouldn't repeat it) !rule 1 hello 2 coc - Show rule 1 (only) !rule 999 coc - Error about invalid indices !rule coc 999 - Error about i...

odd spireBOT
regal archBOT
regal archBOT
odd spireBOT
odd spireBOT
odd spireBOT
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
regal archBOT
#

There are a couple of reasons I can for only matching at the start:

  • Acts more like other commands, in that any text after the command invocation is ignored.
  • Easier to understand how the command works (e.g. someone wouldn't have to guess which word in someones message is causing the rule to appear)
  • Avoids false positives (e.g. !rule coc While this isn't an advert, it *does* break our code of conduct won't match advert)
  • If we match through the whole message people would probably...
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/forms-backend] New branch created: poetry\-1\.2
#

Poetry 1.2 introduced a regression which broke pip --user installs. These types of install were the main way we did installations in docker and CI, as they made it much more convenient to control the location, availability, and caching of packages.

Poetry's team does not recognize this as a supported use case, so major changes were required to get everything working again. Most of the changes were consolidated into chrislovering/python-poetry-base for docker, and HassanAbouelela/setup-py...

odd spireBOT
regal archBOT
#
[python-discord/king-arthur] New branch created: poetry\-1\.2
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

Poetry 1.2 introduced a regression which broke pip --user installs. These types of install were the main way we did installations in docker and CI, as they made it much more convenient to control the location, availability, and caching of packages.

Poetry's team does not recognize this as a supported use case, so major changes were required to get everything working again. Most of the changes were consolidated into chrislovering/python-poetry-base for docker, and HassanAbouelela/setup-py...

odd spireBOT
regal archBOT
#
[python-discord/sir-lancebot] New branch created: poetry\-1\.2
#

Relevant Issues

Approved by myself and @ChrisLovering

<!-- Link the issue by typing: "Closes #" (Closes #0 to close issue 0 for example). -->

Description

Poetry 1.2 introduced a regression which broke pip --user installs. These types of install were the main way we did installations in docker and CI, as they made it much more convenient to control the location, availability, and caching of packages.

Poetry's team does not recognize this as a supported use case, so ma...

odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
#

Poetry 1.2 introduced a regression which broke pip --user installs. These types of install were the main way we did installations in docker and CI, as they made it much more convenient to control the location, availability, and caching of packages.

Poetry's team does not recognize this as a supported use case, so major changes were required to get everything working again. Most of the changes were consolidated into chrislovering/python-poetry-base for docker, and HassanAbouelela/setup-py...

#
[python-discord/sir-robin] branch deleted: poetry\-1\.2
#
[python-discord/sir-lancebot] branch deleted: poetry\-1\.2
thin oysterBOT
#
Sir Robin

Connected!

regal archBOT
#
[python-discord/metricity] branch deleted: poetry\-1\.2
night lilyBOT
#
Sir Lancebot

Connected!

odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
#

Looking at https://pypi.org/pypi/base2048/json, the summary attribute is null.

We're passing this summary attribute straight into discord.py's escape_markdown, which is passing into re.sub, which can only take str|bytes -
https://github.com/python-discord/bot/blob/4dc9a952a71d03959e302434e8e9941e9bd3b577/bot/exts/info/pypi.py#L57

PyPi doesn't require the summary - we don't want to fail every time sometime forgets to add one...
Can we just fix this by converting the `None...

regal archBOT
regal archBOT
odd spireBOT
regal archBOT
#
[python-discord/bot-core] New branch created: bump\-d\.py
regal archBOT
#
[python-discord/bot-core] branch deleted: poetry\-1\.2\.0
regal archBOT
odd spireBOT
odd spireBOT
odd spireBOT
regal archBOT
#

There are a couple of cases that have come up that I didn't consider in my original proposed specification #2108 (comment). Here are some updated cases, and how I think they should work (all still open to discussion).

!rule 1 coc 2 - Show rule 1 and 2 (coc is rule 1, shouldn't repeat it) !rule 1 hello 2 coc - Show rule 1 (only) !rule 999 coc - Error about invalid indices !rule coc 999 - Error about i...

#
[python-discord/bot] New branch created: 2278\-pypi\-cmd\-fix
regal archBOT
#
[python-discord/bot] branch deleted: disable\-pytest\-nose\-plugin
#

40ab60f Allow referencing message as argument to !remi... - TizzySaurus [0f4bc18](https://github.com/python-discord/bot/commit/0f4bc18ceb5ef5e55e003da409a3da8e3e1d9cf8) Disable nose plugin in pytest - wookie184 [29d8820](https://github.com/python-discord/bot/commit/29d882065e12f9a042b14d6dfaf3161af215c953) Make reference message in reminders italic. - TizzySaurus [2536152](https://github.com/python-discord/bot/commit/2536152ffe43a9058dffc03fd8939015c0fd3d37) Merge branch 'main' into bot-2231-bug - TizzySaurus [350df44`](https://github.com/python-discord/bot/commit/350df44731875c4e8b28fc509d83b431cfefbc5c) Use venvs with poetry in Dockerfile - ChrisLovering

#

I think it's slightly neater here to use the start argument of enumerate, and I think rule_number is a more descriptive name than pick.

        for keyword in keywords:
            for rule_number, rule in enumerate(full_rules, 1):
                if keyword in rule[1]:
                    final_rule_numbers.append(rule_number)
                    break

Where you create available_keywords above you could instead consider making a keyword_to_rule_number di...

odd spireBOT
regal archBOT
#
[python-discord/bot] branch deleted: fix\-ensure\-cached\-claimant\-none\-check
#
[python-discord/bot] branch deleted: fix\-not\-awaited\-coroutine\-warning
oak estuaryBOT
odd spireBOT
oak estuaryBOT
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
oak estuaryBOT
odd spireBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
regal archBOT
odd spireBOT
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
#
[python-discord/bot] branch deleted: 2278\-pypi\-cmd\-fix
odd spireBOT
oak estuaryBOT
odd spireBOT
regal archBOT
#

Avoid using get() if you don't need to rely on its default value. Otherwise, it may mislead readers into thinking that sometimes the key may not exist. Furthermore, the code below assumes everything in final_rule_numbers is a valid index. If a bug was somehow introduced that caused a key to not exist in keyword_to_rule_number, the resulting error would be clearer if it originated from this point than from somewhere below.