#dev-contrib

1 messages · Page 1 of 1 (latest)

vale ibex
#

Is timeit no longer working?

#

I'm not sure how/if my changes broke it

#

or if I'm just misunderstanding how it's supposed to work

#

!timeit ```py
import time
time.sleep(4)
print("hello")

stable mountainBOT
#

@vale ibex :warning: Your 3.11 timeit job timed out or ran out of memory.

[No output]
vale ibex
#

Is this supposed to say how long it took?

#

oh, timeout/OOM??

brazen charm
#

!timeit 0

stable mountainBOT
#

@brazen charm :white_check_mark: Your 3.11 timeit job has completed with return code 0.

20000000 loops, best of 5: 14.3 nsec per loop
brazen charm
#

Seems fine? Sleeping 4 sec probably takes too long when it loops

vale ibex
#

ahhh right, yea ok now I also understand why it fails

#

ignore me then, it was my misunderstanding of how it worked

brazen charm
#

Could snekbox differentiate between oom kill and a timeout to make that clearer?

vale ibex
#

NSJail uses sigkill for both, not sure if we can get the specific reason from nsjail or not

#

it seems to use the same return code for both

tawdry vapor
#

It's possible by parsing the log

static canyon
#

Is there a specific reasoning to not being able to use !remind in topic channels or ot?

#

I'd think the only place you shouldn't be able to use it is the Occupied Help Channels category since the message could be sent when a channel is closed or even worse interrupt someone else's help (and maybe also shouldn't in pygen since it can be so fast moving)

gritty wind
#

The idea is you can't predict when the messages will appear and under what conditions. The channel could be entirely dead, or it could interrupt an on-going conversation

#

In that aspect, it's better to run it in a channel where that doesn't matter like #bot-commands

static canyon
#

Right, fair enough

#

Also, how do you do bot-core testing? I'm trying to run it with docker-compose run --build but keep getting file dir issues since the docker-compose.yml is in /dev instead of /

#

Do I just move the file into /?

gritty wind
#

No the docker compose is broken right now

#

Not that it would matter, the image just installs dependencies

#

We haven't nailed down an official testing method, but I have a bare bones bot which I just keep in the project repo and test features with

#

You'd need to write some code anyway, so a final solution wouldn't look too different, we'd just ship the boilerplate

#

That and unittests, but we don't enforce them too tightly right now

static canyon
# 
async def clean_text_or_reply(ctx: Context, text: Optional[str] = None) -> Optional[str]:
    """Returns cleaned version of `text`, if given, else referenced message, if found, else `None`."""
    clean_content_converter = clean_content(fix_channel_mentions=True)

    if text:
        return await clean_content_converter.convert(ctx, text)

    if (
        (replied_message := getattr(ctx.message.reference, "resolved", None))  # message has a cached reference
        and isinstance(replied_message, Message)  # referenced message hasn't been deleted
    ):
        return await clean_content_converter.convert(ctx, ctx.message.reference.resolved.content)

    # No text provided, and either no message was referenced or we can't access the content
    return None
```for reference, this is what I'm testing (bot-core#100)
dusky shoreBOT
gritty wind
#

Like we can include something like:

from discord import bot
bot = Bot()

@bot.command()
async ...

bot.run(token)

But beyond that, what you need is entirely dependent on what feature you're testing

#

So we couldn't really include a way to run that's anything more than this template

static canyon
#

Right yeah, fair enough

gritty wind
#

For your converter, you could use it in a command and make sure it works

#

If you like, you could also install the project into bot and test there directly

#

From the bot's venv: pip install /path/to/botcore -e

#

After that it'll exist and stay up to date with your changes so you can use it in bot

#

I haven't tested, so I'm not sure if it's compatible with the editable option, or if you need to run install again each time

static canyon
#

Cool, thanks

static canyon
static canyon
# gritty wind Like we can include something like: ```py from discord import bot bot = Bot() @...

I don't seem to be able to get the bot to start```py
from os import getenv

import discord
from discord.ext.commands import BadArgument, Bot
from dotenv import load_dotenv

from botcore.utils.commands import clean_text_or_reply

bot = Bot(command_prefix='~', intents=discord.Intents.default())

@bot.command()
async def test(ctx, text=None):
cleaned_text = await clean_text_or_reply(ctx, text)

if not cleaned_text:
    raise BadArgument("Missing text/reply for parameter `text`")

await ctx.send(f"Here's your cleaned text: `{cleaned_text}`")

load_dotenv()
bot.run(getenv("BOT_TOKEN"))

C:\Users\tizzy\AppData\Local\pypoetry\Cache\virtualenvs\bot-core-vZsMerCK-py3.9\Scripts\python.exe C:/Users/tizzy/bot-core/testbot.py
[2022-07-23 09:07:09] [INFO    ] discord.client: logging in using static token
[2022-07-23 09:07:09] [INFO    ] discord.gateway: Shard ID None has sent the IDENTIFY payload.
[2022-07-23 09:07:09] [INFO    ] discord.gateway: Shard ID None has connected to Gateway (Session ID: 42af6a97863f09ebcdfe93c2fd314770).
```
#

I get those 3 log messages and that's it

#

And the command/bot doesn't respond

gritty wind
#

Lol I was just writing a script which looks identical to this

#

Ok let's see uhh do you need the message intent for this?

#

Is that enabled by default

static canyon
#

Ah yeah, I need the message intent probably

#

Don't think that's default

static canyon
# 
from os import getenv

import discord
from discord.ext.commands import BadArgument, Bot
from dotenv import load_dotenv

from botcore.utils.commands import clean_text_or_reply

intents = discord.Intents.all()
intents.presences = False

bot = Bot(command_prefix='~', intents=intents)


""""@bot.event
async def on_message(message):
    print(message.content)
    await bot.process_commands(message)
"""

@bot.command()
async def test(ctx, text=None):
    cleaned_text = await clean_text_or_reply(ctx, text)

    if not cleaned_text:
        raise BadArgument("Missing text/reply for parameter `text`")

    await ctx.send(f"Here's your cleaned text: `{cleaned_text}`")


load_dotenv()
bot.run(getenv("BOT_TOKEN"))
#

The on_message runs when I uncomment it so the bot's seeing the message, just not running the command for some reason

#

Wait I did the wrong prefix 🤦

gritty wind
#

btw why are we using this as a function instead of a converter?

static canyon
gritty wind
#

Because of the ctx?

static canyon
#

Since text is optional, and a converter parameter cannot be optional

gritty wind
#

Does typing.Union or optional not work for that?

static canyon
#

Nope

#

You'd have to do something likepy async def test(ctx, text: str = None): await Converter().convert(ctx, text)but we decided at that point to just use a util

gritty wind
#

I see

static canyon
#

bot-core#101, bot#2220 and bot#2031 are all ready for reviews now if anyone's free 😄

dusky shoreBOT
gritty wind
#

@vale ibex why is allowed roles explicitly separated from all the kwargs in the BotBase init? It's just passed to the super init with all the other kwargs anyway, so it seems redundant. It's also mandatory for some reason

vale ibex
gritty wind
#

Alright thanks

gritty wind
#

Ok I've written some pretty nice boilerplate now for bot-core, I'll push that as a PR

#

I've written an md file for it, wonder if it can be included in the docs. I think sphinx or one of the hundred extensions does support md

gritty wind
#

@static canyon we're good to merge now I think. We just need to include some changelog stuff and bump version, would you like to do that?

static canyon
gritty wind
#

The changelog format is basically ```

  • :type:PR number Description
#

Type and PR number you already have: feature & 101

#

Description can be anything, it just needs to be descriptive enough to get a general idea of what changed

#

For instance: Add a utility to get text from message contents or replies

#

This would go at the very top of the changelog.rst file under the changelog title

#

Right above it, you'd include a similar entry to this, but with type release. That takes the form ```

  • :release:x.y.z <23rd July 2022>
#

Where x.y.z is the next version number according to semver

static canyon
#

Right, okay

gritty wind
#

Our current pyproject is on 7.4.0, so the next feature release is 7.5.0

static canyon
#

Do I do this under the same branch?

gritty wind
#

Yeah all in the same PR it's fine

#

Let me actually check with chris first, since he's claimed 7.4 for a beta

#

Hmm, let's merge it without a release, then we can cut a release once chris' beta is done

#

So it would just be the changelog feature

static canyon
#

Aight

gritty wind
#

Thanks

static canyon
#

So the only thing I add is```

  • :feature:100 Add a utility to clean a string or referenced message's content```?
gritty wind
#

Yes please

#

Err 101

static canyon
#

Right yeah

#

100 is the issue

gritty wind
#

Ah

static canyon
#

Is that good? Just pushed

#

Hmm actually

#

Would it be better to make the function take a Message instead of the Context?

#

Nvm, it needs to have Context for the converter

gritty wind
#

Yeah

#

This is perfect, thanks

static canyon
#

👍

gritty wind
#

I can merge it now if you're ready

static canyon
#

Yep

gritty wind
#

If it doesn't see a release soon, feel free to poke me about it

static canyon
#

Aight

static canyon
stable mountainBOT
#
Yeah okay.

Your reminder will arrive on <t:1658587673:F>!

ID: 4841
static canyon
#

!reminder cancel 4841

stable mountainBOT
#
Sure.

That reminder has been deleted successfully!

ID: 4841
gritty wind
#

It's going to be live on docs, but you can't install it on botcore since we didn't cut an actual release

#

(That process would involve the things I mentioned above and adding a git tag)

#

Let me see what the blocker is on that beta version

vale ibex
#

7.4.0 released fully

#

I just never removed the tag

gritty wind
#

Oh

#

That works then

#

I can do it after dinner

vale ibex
#

Do we want to get into the habit of forcepushing beta tags away?

#

we could just leave them

gritty wind
#

You wouldn’t push it would you? Just a normal push

#

Anyways it doesn’t matter it doesn’t do any harm

vale ibex
#

Ah yea, it would just be a git push --delete

#

only need forcepush to change the commit it references

gritty wind
#

We're almost out of PRs to review

#

Chris write more PRs

vale ibex
#

don't worry, you'll have your 3.10 PR shortly

#

shortly in the relative sense

gritty wind
#

lmfao

#

Review my PR first or I will report you to the open source authorities

vale ibex
#

Sorry, I only review the PRs of people who have 10+ years of experience with bot-core, no exceptions and bot doesn't count

gritty wind
#

I just disabled the branch protection to push the version bump

#

And I'll do it again don't you test me

vale ibex
#

you need to do that to push tags?

gritty wind
#

Nah but changelog and pyproject

vale ibex
#

ahhh lol

#

wait lol

#

I just rebased a dependabot pr, didn't see you already did the other

gritty wind
#

Oh it's a race now 👁️

vale ibex
#

it's hardly a race if I can cancel your actions

gritty wind
#

Lmao

#

Re-request review lol

#

We're almost at 100 PRs now, wonder how many are dependabot

#

53

#

Are we even writing any code lol

vale ibex
#

winner

unknown.png
gritty wind
#

The game is rigged

#

Chris bought off the judges

#

I was going to unironically rebase the pre-commit PR

#

I wrote it and locked it, but it completely rewrote the lockfile because poetry can't agree on the order of the dependencies

#

I probably just need to update poetry

#

Why did you review, I was rebasing :O

vale ibex
#

lol

gritty wind
#

I looked up sphinx tables

#

Half the results were actual physical tables

#

The other half were the shittiest imaginable syntax possible

#

I don't know which one is less helpful

vale ibex
gritty wind
#

You requested it ✅

#

Nah it's unrelated

#

My last commit before the push had a typo

#

I force pushed the typo away

vale ibex
#

oh lol

gritty wind
#

Everything before is unchanged

#

You should know this, you wrote this commit after all

vale ibex
#

Signed-off-by: Hassan Abouelela <hassan@hassanamr.com> all blame is on this personn

gritty wind
#

who's hassanamr? Never heard of em

#

Now for the actual rebase

vale ibex
gritty wind
#

Lmfaoooooo this is

#

wow ok

#

Alright I've rebased this and I'll push it but let's not cut a release for it

#

We'll bundle it with the next feature

#

Or 3.10

vale ibex
#

sgtm

static canyon
#

bot#2089

dusky shoreBOT
static canyon
#

Bluenix suggested continuing it as an ephemeral, but not really sure how to do that

molten perch
#

Let me check the code.

stable mountainBOT
#

bot/exts/utils/reminders.py lines 344 to 348

try:
    m = await partial_message.reply(
        content=f"{additional_mentions}",
        embed=embed,
        view=snooze_button_view```
molten perch
#

Oh, wait. No. The response snooze_button_view sends.

#

On second thought, I was right.

static canyon
#

Just ephemeral=True?

molten perch
#

Oh, wait. That's a partial message.

#

Let me think.

#

Okay, so one thing you could do, is instead of editing the message (the one that the bot sends with using the snippet I sent) you send another message with the Dropdown.

static canyon
#

So I change L150 await interaction.response.edit_message(view=snooze_select_view) to instead send a new message?

molten perch
#

Yes, to an ephemeral one.

static canyon
#

So send_message instead of edit_message?

molten perch
#

And maybe after clicking the Snooze button, you could disable the button.

static canyon
molten perch
#

Yep, yep.

static canyon
molten perch
stable mountainBOT
#

bot/exts/utils/reminders.py line 157

self.parent_view.reminder_was_snoozed = True```
static canyon
#

Errr no

static canyon
#

I forgot my line numbers are different because I've made other changes

molten perch
#

Not exactly, it stops "listening" to the view, but the buttons will still be "clickable"

static canyon
#

So how do I disable it completely?

molten perch
#

So when you click on that you get "Interaction Failed"

static canyon
molten perch
#

Well, actually a new one.

static canyon
#

Yeah

#

Okay, cool

#

I think I get that

static canyon
static canyon
molten perch
# static canyon Now just this ^

Actually the problem with that, is that you'd have to set the disabled attribute to True which means you'd have to respond with a new view, so I don't thank that's feasible that way, correct me if I'm wrong though.

static canyon
#

Hmm

#

I think you can get away with it if I move it to the select_snooze_duration()

#

I'll mess around with it, thanks

molten perch
#

Yeah, no worries.

static canyon
#

It's fine where it is

#

Because of the .wait()

# 
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/discord/client.py", line 847, in runner
    await self.start(token, reconnect=reconnect)
  File "/usr/local/lib/python3.9/site-packages/discord/client.py", line 780, in start
    await self.login(token)
  File "/bot/bot/bot.py", line 258, in login
    await self.stats.create_socket()
  File "/bot/bot/async_stats.py", line 29, in create_socket
    self._transport, _ = await self._loop.create_datagram_endpoint(
  File "/usr/local/lib/python3.9/site-packages/discord/client.py", line 110, in __getattr__
    raise AttributeError(msg)
AttributeError: loop attribute cannot be accessed in non-async contexts. Consider using either an asynchronous main function and passing it to asyncio.run or using asynchronous initialisation hooks such as Client.setup_hook

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  ...
    await self.start(token, reconnect=reconnect)
  File "/usr/local/lib/python3.9/asyncio/locks.py", line 226, in wait
    await fut
RuntimeError: Task <Task pending name='Task-3' coro=<Client.run.<locals>.runner() running at /usr/local/lib/python3.9/site-packages/discord/client.py:847> cb=[_run_until_complete_cb() at /usr/local/lib/python3.9/asyncio/base_events.py:184]> got Future <Future pending> attached to a different loop
ERROR: 1```Any idea why I get this error when doing `docker-compose run bot`? @molten perch
molten perch
stable mountainBOT
#

bot/exts/utils/reminders.py line 169

scheduling.create_task(self.reschedule_reminders(), event_loop=self.bot.loop)```
static canyon
#

hmmm

molten perch
#

Let me give you an example how to resolve this issue.

static canyon
#

yes please lol

#

wait_until_ready maybe?

molten perch
#

No, no there is a new async init method for cogs, but I keep forgeting the name

#

there is a gist on the topic I'm trying to find it lol

static canyon
#

ah lol

molten perch
static canyon
#

So I don't create_task(), I just create a cog_load method?

#

Hmm

molten perch
#

Okay, I might be totally wrong lol. Haven't gotten used to this change.

static canyon
#

That seems to be when you do bot.loop.create_task but we're doing scheduling.create_task here, just referencing bot.loop

#

The only thing I can think of is moving that line into a async def cog_load

#

Whiiicch doesn't work lol

molten perch
# 

async def setup(bot):
    await bot.add_cog(MyCog(bot))

add cog is now async.

static canyon
#

Ah right, yea

#

Nope, same error still

molten perch
#

Did you move the create_task back to init?

static canyon
# 
AttributeError: loop attribute cannot be accessed in non-async contexts. Consider using either an asynchronous main function and passing it to asyncio.run or using asynchronous initialisation hooks such as Client.setup_hook

During handling of the above exception, another exception occurred:

RuntimeError: Task <Task pending name='Task-3' coro=<Client.run.<locals>.runner() running at /usr/local/lib/python3.9/site-packages/discord/client.py:847> cb=[_run_until_complete_cb() at /usr/local/lib/python3.9/asyncio/base_events.py:184]> got Future <Future pending> attached to a different loop```
static canyon
# 
class Reminders(Cog):
    """Provide in-channel reminder functionality."""

    def __init__(self, bot: Bot):
        self.bot = bot
        self.scheduler = Scheduler(self.__class__.__name__)

    async def cog_load(self):
        scheduling.create_task(self.reschedule_reminders(), event_loop=self.bot.loop)


async def setup(bot: Bot) -> None:
    """Load the Reminders cog."""
    await bot.add_cog(Reminders(bot))```is what I've got at the moment
#

I'm a bit lost

vale ibex
#

you should rebase onto main, the async cog_load logic is already there

molten perch
#

Yeah, yeah. So I was a tiny bit wrong. You can still access self.bot.loop, what I was thinking of is Client.loop

static canyon
vale ibex
#

I can't say you won't need to change anything since I'm not sure what's already in your PR

#

All I'm saying is that the Reminders cog has already been rewritten to remove the create_task on load

static canyon
#

Right

#

I see

static canyon
vale ibex
#

either will work, depends on what you want your history to look like in the branch

#

merge will add a merge commit to your branch, with all the changes. Rebasing will move all your commits "up the tree" so it's as if you started (based) your branch from the current version of main

static canyon
#

Right, I see

#

Is there a particular preference?

vale ibex
#

now that's a hotly debated topic 😅

static canyon
#

heh

vale ibex
#

I usually rebase unless another person is already working on the same branch

#

or if PR comments have already been made

#

rebasing messes up existing PR comments

static canyon
vale ibex
#

👍

molten perch
#

What I'm not sure about, is if your branch was based of the main branch in February, why won't it work with the initial settings 🤔

static canyon
#

Wait I just realised something

static canyon
#

I need like git merge main or something

molten perch
#

If you do git merge main you should do git fetch before, but just doing git pull is probably easier. (Ofc. with the proper parameters)

#

But, I'm sure you figured it out, by the time I answered 😅

static canyon
#

I instead went down a rabbit hole of ruining my git log

#

But I'm back now lol

static canyon
molten perch
#

well, since you're merging git pull origin main

static canyon
#

right, okay

molten perch
#

You might have to set a "pull strategy"

static canyon
# 
 * branch              main       -> FETCH_HEAD
error: The following untracked working tree files would be overwritten by merge:
        bot/exts/info/resources.py
        bot/exts/utils/thread_bumper.py
        bot/resources/tags/dashmpip.md
        bot/resources/tags/regex.md
        bot/resources/tags/type-hint.md
        tests/bot/exts/moderation/test_clean.py
Please move or remove them before you merge.
Aborting
#

What do I do?

#

@molten perch

molten perch
#

I think the easiest solution would be just to re-clone the repo and pull main, up-front.

#

But, blindly I can't really say anything for sure

vale ibex
#

That error is saying that the files listed you have got uncommited changes to.

#

So if you pull, it will overwrite those uncommitted changes

static canyon
#

Ah, I see

#

If I don't care about that, do I just git pull origin main --force?

vale ibex
#

git reset --hard origin/main

#

make sure you have checked out the main branch

static canyon
#

Hmm

static canyon
#

wrong reply, meant your command

vale ibex
#

Yea, checkout main and run that

#

then checkout your feature branch and merge in main

static canyon
#

I can't checkout because there's differences

vale ibex
#

can't checkout main?

static canyon
vale ibex
#

I do not know what IDE you're using or what action causes that.

#

Are you trying to checkout main?

static canyon
#

PyCharm

static canyon
vale ibex
#

Alright, then you can stash your changes on that branch and then checkout main

static canyon
#

stash?

vale ibex
#

Yea, run git stash while you're on your branch

#

that will save your uncomitted changes on that branch to a stash

static canyon
#

No local changes to save
rofl

#

Gotta love git

vale ibex
#

I don't think this is a git problem :)

#

could you run git status?

static canyon
# 
PS C:\Users\tizzy\bot> git status
On branch add-reminder-snoozing
Your branch is ahead of 'origin/add-reminder-snoozing' by 2 commits.
  (use "git push" to publish your local commits)

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        bot/exts/info/resources.py
        bot/exts/utils/thread_bumper.py
        bot/resources/tags/dashmpip.md
        bot/resources/tags/regex.md
        bot/resources/tags/type-hint.md
        tests/bot/exts/moderation/test_clean.py

nothing added to commit but untracked files present (use "git add" to track)
vale ibex
#

alright, do git stash push -u

static canyon
#

okay, done

#

So now checkout main and do the command?

vale ibex
#

what does git status look like now?

static canyon
# 
PS C:\Users\tizzy\bot> git status
On branch add-reminder-snoozing
Your branch is ahead of 'origin/add-reminder-snoozing' by 2 commits.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean
vale ibex
#

cool

#

yea, checkout main, reset to origin/main

static canyon
# 
PS C:\Users\tizzy\bot> git reset --hard origin/main
HEAD is now at 31110a78 Merge pull request #2220 from python-discord/fix-bot-2168
```done
vale ibex
#

then checkout this branch, run git stash pop to re-apply the stash and then git merge main to merge in main

static canyon
# 
PS C:\Users\tizzy\bot> git merge main
error: The following untracked working tree files would be overwritten by merge:
        bot/exts/info/resources.py
        bot/exts/utils/thread_bumper.py
        bot/resources/tags/dashmpip.md
        bot/resources/tags/regex.md
        bot/resources/tags/type-hint.md
        tests/bot/exts/moderation/test_clean.py
Please move or remove them before you merge.
Aborting
#

lol

#

I did the pop first

vale ibex
#

ah alright, stash them again and pop after the merge

static canyon
#

Let's see...

static canyon
vale ibex
#

you can always delete them if you don't need them anymore

static canyon
#

aight

#

git log seems to be a little cursed now but it works

#

or more likely I'm forgetting how git log ordering works

vale ibex
#

a merge commit will mean all the commits since you made your commits will be above your commits in the tree

#

it's orders on commit date

static canyon
#

Right, yeah

#

It's fine then

#

ERROR: Service "snekbox" was pulled in as a dependency of service "bot" but is not enabled by the active profiles. You may fix this by adding a common profile to "snekbox" and "bot".
lemon_pensive

vale ibex
#

A rebase re-writes the commits in your branch to have a new commit date, so they'll be further up

#

Yea, run docker compose --profile 3.10 up bot

#

or just docker compose up

#

I've got a fix for that issue in my PR atm

static canyon
#

PS C:\Users\tizzy\bot> docker-compose up
ERROR: Service "snekbox" was pulled in as a dependency of service "bot" but is not enabled by the active profiles. You may fix this by adding a common profile to "snekbox" and "bot".

vale ibex
#

Yea, you'll need to do the first one then

static canyon
#

Things are building

vale ibex
#

You can remove the bot on the end if you want all logs

static canyon
#

Yeah it's running, thanks

# 
 discord.ext.commands.errors.ExtensionFailed: Extension 'bot.exts.utils.thread_bumper' raised an error: ResponseCodeError: Status: 404 Response:```can this be ignored?
vale ibex
#

is there more to the error?

static canyon
#

yeah, gimme a sec

#

it's a whole html thing

static canyon
#

the bot's running so I should be good

vale ibex
#

run docker compose pull

#

your web image is out of date

static canyon
#

ah right

#

fair enough

#

I'll do that next start

#

Another tracebackpy bot_1 | Traceback (most recent call last): bot_1 | File "/usr/local/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 917, in _load_from_module_spec bot_1 | spec.loader.exec_module(lib) # type: ignore bot_1 | File "<frozen importlib._bootstrap_external>", line 850, in exec_module bot_1 | File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed bot_1 | File "/bot/bot/exts/utils/snekbox.py", line 10, in <module> bot_1 | from botcore.utils import interactions bot_1 | ImportError: cannot import name 'interactions' from 'botcore.utils' (/usr/local/lib/python3.9/site-packages/botcore/utils/__init__.py) bot_1 | bot_1 | The above exception was the direct cause of the following exception: bot_1 | bot_1 | Traceback (most recent call last): bot_1 | File "/usr/local/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 995, in load_extension bot_1 | await self._load_from_module_spec(spec, name) bot_1 | File "/usr/local/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 920, in _load_from_module_spec bot_1 | raise errors.ExtensionFailed(key, e) from e bot_1 | discord.ext.commands.errors.ExtensionFailed: Extension 'bot.exts.utils.snekbox' raised an error: ImportError: cannot import name 'interactions' from 'botcore.utils' (/usr/local/lib/python3.9/site-packages/botcore/utils/__init__.py)

#

bot keeps running though

vale ibex
#

What version of botcore are you using atm?

#

in the pyproject

static canyon
#

7.4.0

vale ibex
#

could you run docker compose pull and then docker compose build

#

then restart the bot

static canyon
#

Ah, fun. Now my code has an errorpy bot_1 | 2022-07-23 21:34:22 | discord.ui.view | ERROR | Ignoring exception in view <SnoozeSelectView timeout=30 children=1> for item <Select placeholder='Select the snooze duration' min_values=1 max_values=1 options=[<SelectOption label='1 minute' value='1 minute' description=None emoji=None default=False>, <SelectOption label='5 minutes' value='5 minutes' description=None emoji=None default=False>, <SelectOption label='15 minutes' value='15 minutes' description=None emoji=None default=False>, <SelectOption label='30 minutes' value='30 minutes' description=None emoji=None default=False>, <SelectOption label='1 hour' value='1 hour' description=None emoji=None default=False>, <SelectOption label='3 hours' value='3 hours' description=None emoji=None default=False>, <SelectOption label='6 hours' value='6 hours' description=None emoji=None default=False>, <SelectOption label='1 day' value='1 day' description=None emoji=None default=False>, <SelectOption label='1 week' value='1 week' description=None emoji=None default=False>, <SelectOption label='1 month' value='1 month' description=None emoji=None default=False>, <SelectOption label='3 months' value='3 months' description=None emoji=None default=False>, <SelectOption label='6 months' value='6 months' description=None emoji=None default=False>, <SelectOption label='1 year' value='1 year' description=None emoji=None default=False>] disabled=False> bot_1 | Traceback (most recent call last): bot_1 | File "/usr/local/lib/python3.9/site-packages/discord/ui/view.py", line 412, in _scheduled_task bot_1 | await item.callback(interaction) bot_1 | File "/bot/bot/exts/utils/reminders.py", line 83, in select_snooze_duration bot_1 | selected_duration = interaction.data["values"][0] bot_1 | AttributeError: 'Select' object has no attribute 'data'

# 
@discord.ui.select(placeholder="Select the snooze duration")
    async def select_snooze_duration(self, _: discord.ui.Select, interaction: discord.Interaction) -> None:
        """Drop down menu which contains a list of snooze durations one can choose."""
        selected_duration = interaction.data["values"][0]```has the order of parameters changed or something?
vale ibex
#

!d discord.ui.select

stable mountainBOT
#
discord.ui.select 

discord.ui.select(*, placeholder=None, custom_id=..., min_values=1, max_values=1, options=..., disabled=False, row=None)```
A decorator that attaches a select menu to a component.

The function being decorated should have three parameters, `self` representing the [`discord.ui.View`](https://discordpy.readthedocs.io/en/latest/interactions/api.html#discord.ui.View "discord.ui.View"), the [`discord.Interaction`](https://discordpy.readthedocs.io/en/latest/interactions/api.html#discord.Interaction "discord.Interaction") you receive and the [`discord.ui.Select`](https://discordpy.readthedocs.io/en/latest/interactions/api.html#discord.ui.Select "discord.ui.Select") being used.

In order to get the selected items that the user has chosen within the callback use [`Select.values`](https://discordpy.readthedocs.io/en/latest/interactions/api.html#discord.ui.Select.values "discord.ui.Select.values").
vale ibex
#

yup

#

was likely changed to be consistent with the slash command impl

static canyon
#

right

static canyon
#

Yeah, that seems to have fixed it

#

Thanks

static canyon
#

I guess I don't need to re-send it

vale ibex
dusky shoreBOT
vale ibex
#

Should I add a our lock_arg deco onto this call, to lock on the DocItem, using the wait arg too

#

Ah, that won't work since lock_arg needs an int or str

#

guess I could use the lock deco and write a function to return a resource_id based on the DocItem attrs

static canyon
stable mountainBOT
#

bot/exts/utils/reminders.py line 309

@lock_arg(LOCK_NAMESPACE, "reminder", itemgetter("id"), raise_error=True)```
vale ibex
#

What's waiting?

#

oh, the snooze

static canyon
stable mountainBOT
#

bot/exts/utils/reminders.py line 362

await snooze_button_view.wait()```
static canyon
#

yeah

vale ibex
#

you could move the delete logic to the view's timeout

static canyon
#

The exact code is changed now but shows rough idea

vale ibex
#

and make it timeout after 5 min

#

that way the function itself isn't waiting, but is triggered on the view timeout

static canyon
#

The view being SnoozeButtonView?

vale ibex
#

Yea

#

you can add a async def on_timeout() function to that view, and it will be called on timeout

static canyon
#

The issue is that I need to do one thing on timeout and another thing when it doesn't timeout

#

Which as far as I know can only be achieved with wait()

timid sentinel
vale ibex
#

which will mean the on_timeout function won't get called

static canyon
#

yeah but how do I run the other code

vale ibex
#

What is the other code?

static canyon
#

the code I want to run when it doesn't timeout

#

i.e. snoozing logic

vale ibex
#

isn't that on button press?

static canyon
#

hmm

#

true

vale ibex
#

There are 3 separate awaits in that set call, so there is some room for concurrency issues

#

I'm going to guess package+group+symbol_id would be enough to serialise a unique identifier for that DocItem

#

so not too difficult to implement our own lock using the existing deco

#

@brazen charm when you get some time, do you happen to know why the namespace locks were required on the DocRedisCache?

#

They were removed in async-rediscache 1.0.0 so I want to know if we need to re-implement locking using our lock deco, or if they're fine without

#

bot#2229 for reference

dusky shoreBOT
brazen charm
#

I guess there's also a potential race at the end where an existing expire may cause the whole hash to expire before a new one is set, but that was there before too

vale ibex
#

I might just add this lock on anyway then

#

it's fairly easy to implement

static canyon
#

Since I don't have access to the scheduler nor the self.send_reminder that gets called at the end of the expiry

#

So it seems I actually can't move that, which in turn means I can't move the on_timeout stuff, which means we're stuck with the 5 minute hang again

vale ibex
#

Can you pass something to the view that gives you access to those things?

static canyon
#

I mean, I can but it'd be super messy

#

I'd be passing the reminder instance itself through 3 levels of classes

#

I can do that but I don't know how I feel about that

#

I can't think of a better way though 🤷

cold island
#

Doesn't the ctx have the cog instance or something?

static canyon
#

I don't have the ctx though

#

The only thing is the bot instance, and that's so that I can do the API calls

#

So I guess I can just make that the Reminders class instance

cold island
#

The view class always felt messy to me, you so often need to make a subclass to take the context, although technically a view isn't tied to a single context

vale ibex
#

I determined that f"{item.package}.{item.symbol_id}" was enough to be a unique_id for that docitem

vale ibex
static canyon
#

I'm gonna push something in a minute so you can see what I've done for now

stable mountainBOT
#

Contribute to Python Discord's Open Source Projects
Looking to contribute to Open Source Projects for the first time? Want to add a feature or fix a bug on the bots on this server? We have on-going projects that people can contribute to, even if you've never contributed to open source before!

Projects to Contribute to
• Sir Lancebot - our fun, beginner-friendly bot
• Python - our utility & moderation bot
• Site - resources, guides, and more

Where to start

  1. Read our contribution guide
  2. Chat with us in #dev-contrib if you're ready to jump in or have any questions
  3. Open an issue or ask to be assigned to an issue to work on
vale ibex
brazen charm
#

was going to say that it may cause issues for empty symbol names. A tuple with the item_key and symbol id should work

vale ibex
#

The ttl is done on the item_key directly well redis_key = f"{self.namespace}:{item_key(item)}" but close enough

#

So we should likely lock on that, rather than allow concurrent sets for individual symbols

brazen charm
#

The second race condition I mentioned would also be nice to fix, though it is out of scope for the pr and nothing immediately comes to mind for it

vale ibex
#

Ahh yea true

#

I didn't fully read that message, clearly

#

I see what you mean now though

brazen charm
#

it should fix itself on the next set call but still is not ideal

static canyon
dusky shoreBOT
gritty wind
#

This is really annoying

#

The site test suite failed on one test where the ID was greater than expected (implying dead data?) but running it again it passed

vale ibex
#

I despise flaky tests

gritty wind
#

It was one of the infraction tests, it expected ID 25 and ended up with 28

#

I don’t have the log anymore

vale ibex
#

I've ran that individual test suite 45 times, and the whole test suite 20 times, and I can't reproduce the error :(

#

Are you running pytest-xdist locally by chance?

gritty wind
#

Nah I’ve dumped those changes completely

#

The only thing that was different from main is that my compose port PR, but that shouldn’t affect it in any way

#

How did you test it 45 times you absolute troll

vale ibex
#

just had it running in a loop on 2 machines lol

gritty wind
#

That is some serious dedication lol

#

I’ll assume something bugged out, maybe a networking error and call it a day

vale ibex
#

Yea

static canyon
#

May I take over bot#2172? It seems the original assignee has gone AWOL.

dusky shoreBOT
static canyon
#

It's a small thing so could possibly even implement it as part of bot#2089

dusky shoreBOT
elder belfry
#

Hey guys 👋
If you got some time, have a look at sir-lancebot#1060, it's fairly a simple PR with plain fixes here and there. I believe anyone who's just starting out like me can also review it lemon_happy

dusky shoreBOT
civic geyser
#

Can I get any help related to Django here?

atomic ivy
civic geyser
scenic moth
#

Hey, if there's any studious discord snek bot devs around I had a thought

#

what if you made a slash command for executing, the first field is code, and other fields are inputs? or just a second optional field is input to be fed so that input()'s would work

atomic ivy
#

you could do that with text commands as well

#

!e currently combines all given code blocks IIRC

vale ibex
#

Yea, the downside of slash commands is that they don't support code blocks, so it can make it hard to actually write you code out

#

We could do soemthing smart with the code block language, if it's marked and input or something then we can pass that as stdin, rather than code

wild prism
#

teaching people to use that seems strictly harder than

# guess = input("Your guess: ")
guess = "12"
gritty wind
#

You could also theoretically switch out stdin with a stringio so you don’t need to modify all the code

#

But most of the time changing it out is the cleanest solution

static canyon
gritty wind
#

I'd give it another couple days

#

That way it's been two weeks

#

Actually why not you can start work now

#

@static canyon you're good to start

static canyon
#

Cool, thanks

static canyon
gritty wind
#

Link?

static canyon
#

It's above

gritty wind
#

Sounds good

static canyon
static canyon
#

I seem to once again be unable to start bot with docker. 2 days ago I was told to do docker-compose --profile 3.10 up bot as a temp fix but now that's not working either (it did 2 days ago)?

#

I get Error response from daemon: network bot_default not found

outer oasis
#

Prune the entire thing and then set it up again?

#

... can you compose up just the network?

atomic ivy
#

idts

static canyon
static canyon
atomic ivy
#

docker-compose down -v
for stopping the services and networks and removing the associated volumes

outer oasis
#

I meant like docker system prune

#

Nuke the whole thing

atomic ivy
#

that would remove way too much stuff pithink

static canyon
atomic ivy
#

what do you get as output from docker-compose up -d bot?

static canyon
#

no such service: snekbox

static canyon
#

But now that's broken too

atomic ivy
#

and what does that output? network not found?

static canyon
# atomic ivy and what does that output? network not found? 
PS C:\Users\tizzy\bot> docker-compose --profile 3.10 up bot
[+] Running 5/4
 - Container bot-postgres-1   Created                                                                              0.1s
 - Container bot-redis-1      Created                                                                              0.1s
 - Container bot-metricity-1  Created                                                                              0.1s
 - Container bot-web-1        Created                                                                              0.1s
 - Container bot-bot-1        Created                                                                              0.1s
Attaching to bot-bot-1
Error response from daemon: network bot_default not found```
#

Yep

thorny obsidian
#

Can you just force re-pull the volumes/images and re-build?

static canyon
outer oasis
#

Wouldn't it be.... docker-compose --build --profile 3.10 up bot

thorny obsidian
#

what does docker pull and then docker compose up --build get you?

atomic ivy
outer oasis
#

ooh fancy

atomic ivy
#

but, it doesn't seem like a container problem to me

static canyon
#

It's building

#

None of these work @outer oasis @atomic ivy @thorny obsidian

atomic ivy
#

what does docker network ls give you?

static canyon
# atomic ivy what does `docker network ls` give you? 
PS C:\Users\tizzy\bot> docker network ls
NETWORK ID     NAME                    DRIVER    SCOPE
a4b558deaf0f   api_default             bridge    local
05bdbb8a90ea   bridge                  bridge    local
2a4da9a78b32   host                    host      local
aca58f6a065b   none                    null      local
7488b1ce02be   pydis-testing_default   bridge    local
cee5f28fbfdc   sir-lancebot_default    bridge    local```
vale ibex
#

Restart docker desktop

#

It does weird things with networks

#

I found restarting the daemon fixed it

static canyon
#

Heh, okay

#

I'll try that

outer oasis
#

docker network create bot_default?

vale ibex
#

You may need to kill the container and then restart

thorny obsidian
#

So looking at similar errors, the fix is to fully restart the daemon. Possibly you need to do a full up prune and then a dead stop to start

atomic ivy
#

truly a d(a)emon

atomic ivy
static canyon
atomic ivy
static canyon
#

The bot's starting up

gritty wind
#

It should be created automatically when doing up, I'm guessing your docker is just freaking out

static canyon
#

Yeah lol

#

RE bot#2172: should I make it so that it shows new content/mentions upon edit too?

dusky shoreBOT
static canyon
#

I guess content isn't sent anyway, but could maybe say how many people it'll now mention

gritty wind
#

I think the timestamp is the most important

#

The other ones seems irrelevant

outer oasis
#

Actually - question about Docker
With the move to Ansible, will Ansible be running the same containers we use now, or will Ansible be installing the bot on bare metal?

gritty wind
#

We'll still be containerized

static canyon
gritty wind
#

So still on docker

static canyon
outer oasis
#

Why not list out the specific mentions?
At least the first x

static canyon
#

We can do though

#

Unless it supports mentioning things other than users

#

Since we only have ids

gritty wind
#

It can mention roles too

static canyon
#

Yeah, then you can't really list them since just from the id you don't know if it's a role or user (and the mention is a different syntax for each)

gritty wind
#

It's really not worth all this effort, since you can already see who you mentioned or what the content is a line above where you edited it

#

But I guess it's fine to add if it's just a line or two

#

Anymore and no

static canyon
#

Yeah, fair enough

#

I'll leave the content since that's not currently included

#

But I'll add the mentions count

outer oasis
#

Oh right it replies to the orignal

static canyon
# 
+       on_success_message = "That reminder has been edited successfully!"

+       if "expiration" in payload:
+           new_expiry_timestamp = time.discord_timestamp(payload['expiration'], time.TimestampFormats.DAY_TIME)
+           on_success_message += f"\n\nYour reminder will now arrive {new_expiry_timestamp}."

+       elif "mentions" in payload:
+           on_success_message += f"\n\nYour reminder will now mention {len(payload['mentions'])} other(s)."

         await self._send_confirmation(
            ctx,
-           on_success="That reminder has been edited successfully!"
+           on_success=on_success_message,
            reminder_id=id_,
        )
```this is the total diff (ignore funky indentation)
outer oasis
#

There's a diff language? TIL

static canyon
#

yep lol

outer oasis
#

Maybe just one blank line so it doesn't get too big?

#

Or is it like Markdown where you have to put two to get the space?

gritty wind
#

This one turned out to be a nice walk in the park after all

#

Ignore the massive diff, that's locking

vale ibex
#

everyline will be reviewed.

gritty wind
#

You got me, I was trying to sneak in an RCE into the deps 😔

vale ibex
#

(after dinner)

static canyon
#

Can I get permission to add reply functionality to editing content (it already exists when creating)

unknown.png
gritty wind
gritty wind
vale ibex
#

I'd suggest limiting the scope of PRs, otherwise it make it much harder to review.

vale ibex
gritty wind
#

Also there doesn't exist a one-to-one relationship between messages and reminders

#

How would that work

static canyon
#

I can make a new PR for "further reminder improvements" if you want

gritty wind
#

Short of full rewrites, it’s better to separate features

static canyon
gritty wind
#

Please don’t call it improvements because that is not a helpful title

#

Instead what the actual improvements is gives more info for reviewers, which should hopefully help get the PRs through quicker

static canyon
#

Fair enough

#

So I'll do a 2nd PR for any further changes?

gritty wind
gritty wind
static canyon
gritty wind
#

Ideally since I’d like to have a clearer idea of the implementation before we start work on it

static canyon
#

Yep, okay

gritty wind
#

I’m not sure how it’d work

static canyon
#

I'll create a new issue for it once I've pushed the current changes to this PR

gritty wind
#

Yup yup

static canyon
#

Aight, pushed to bot#2089

dusky shoreBOT
static canyon
#

I'll create the issue now

static canyon
#

bot#2231

dusky shoreBOT
gritty wind
#

Left a comment on the issue

#

@static canyon I don't really follow what you're saying

#

What's the new behavior that new would get?

static canyon
stable mountainBOT
#
Sure thing!

Your reminder will arrive on <t:1658776820:F>!

ID: 4858
stable mountainBOT
static canyon
stable mountainBOT
#
Yeah okay.

Your reminder will arrive on <t:1658776825:F>!

ID: 4859
stable mountainBOT
static canyon
#

That ^

gritty wind
#

Yes, I think this is fine, not sure how it comes into the edit idea

static canyon
#

Make it so that you can do that for !remind edit content

gritty wind
#

Yes, but that's not actually technically possible

static canyon
#

It is though? Same logic?

gritty wind
#

remind new doesn't need to figure out the state

#

It just makes a new reminder

#

How do you tie a reminder to a replied message like that

gritty wind
stable mountainBOT
#
I got you.

Your reminder will arrive on <t:1658812893:F>!

ID: 4860
gritty wind
stable mountainBOT
#
Can do!

Your reminder will arrive on <t:1658848900:F>!

ID: 4861
static canyon
#

Right, what I mean is the content will get updated to the reference message's content

gritty wind
#

If I edited it now, which would edit

static canyon
#

So !remind edit content _id_ {reference_message_for_content_argument}

gritty wind
#

I see, so it'd just overwrite the body

#

That sounds ok

static canyon
#

Yeah

#

Sorry, didn't explain that very well lemon_sweat

#

And thinking about it the API call one seems kinda redundant so considering it'd cost API calls I'm happy to leave that

gritty wind
#

Like our API calls are pretty quick, so it wouldn't matter if you think it's a good feature

#

It might be, I never encountered that scenario

static canyon
#

!remind 10M hello world

stable mountainBOT
#
You got it!

Your reminder will arrive on <t:1658777665:F>!

ID: 4862
static canyon
#

!remind edit content 4862 hello world

stable mountainBOT
#
Aye aye, cap'n!

That reminder has been edited successfully!

ID: 4862
static canyon
#

Yeah, I don't know, it just seems weird to say "edited successfully" when it wasn't actually changed

#

I think it'd be nice to have if we don't care about API calls

#

Guess I'll let someone else weigh-in and see what they think, whilst I work on the other parts

static canyon
# 
@lock_arg(LOCK_NAMESPACE, "id_", raise_error=True)
    async def delete_reminder(self, ctx: Context, ids: Greedy[int]) -> None:```I have an issue whereby because I've changed the parameter name, the `lock_arg` decorator is now giving an error that `id_` parameter doesn't exist. Can I just change that to `ids`, or will that not work since it's now a list of things to lock?
gritty wind
#

It wouldn't work

#

Can't remember, does it support a callabale

#

Actually even if it does, I don't think our current lock will manage that in any way

#

You might need to write a custom lock for this

static canyon
#

I have no clue how locks works

vale ibex
gritty wind
#

Yes, but it won't work with multiple IDs

vale ibex
#

nope

#

you'd need a lock for each id

#

rather than 1 lock for the whole set

gritty wind
#

We could make a fairly rudimentary lock that works for this, but the lock we have now is actually pretty invovled

#

Might just be best to let that idea rest

#

Or make the wrapped function a util, and have the command iterate through them and call it with each

static canyon
#

Do we really need a lock when we're deleting?

gritty wind
#

Yes, it could cause some nasty bugs, especially if we're considering functions which do multiple operations within a single lock

#

Like you were proposing 😅

static canyon
#

Fair enough

static canyon
gritty wind
# 
@lock_arg(LOCK_NAMESPACE, "id")
async def delete_reminder_util(self, ctx, id) -> None:
  ...

@command()
async def delete_reminder(self, ctx, ids):
  for id in ids:
    await delete_reminder_util(ctx, id)
static canyon
#

Hmm

gritty wind
#

You'd need some error handling logic here

static canyon
# 
def lock_arg(
    namespace: Hashable,
    name_or_pos: function.Argument,
    func: Callable[[Any], _IdCallableReturn] = None,
    *,
    raise_error: bool = False,
    wait: bool = False,
) -> Callable:
    """
    Apply the `lock` decorator using the value of the arg at the given name/position as the ID.

    `func` is an optional callable or awaitable which will return the ID given the argument value.
    See `lock` docs for more information.
    """
    decorator_func = partial(lock, namespace, raise_error=raise_error, wait=wait)
    return function.get_arg_value_wrapper(decorator_func, name_or_pos, func)```this is what `lock_arg` currently looks like. Could I just do```py

@command()
async def delete_reminder(self, ctx, ids):
    for id_ in ids:
        decorator_func = partial(lock, LOCK_NAMESPACE, "id_" raise_error=True)
        function.get_arg_value_wrapper(partial(decorator_func, name_or_pos, None))```
#

If I import lock and function

#

I feel like that would work, but maybe not

gritty wind
#

Why the partial, and what do you plan to do with this? If you want to invoke lock manually without the decorator, could just call it directly at that point

static canyon
gritty wind
#

lock not lock_arg

#

But I'm still not sure what you're proposing

#

What's the next step after obtaining the lock in this fashion, just run the function?

static canyon
#

I'm basically trying to call the decorator on each id_ and hoping that'll just work

#

Maybe even have something like ```py
@command()
async def delete_reminder(self, ctx, ids):
for id_ in ids:
self.delete_reminder(ctx, id)

@lock_arg(LOCK_NAMESPACE, "id_", raise_error=True)
def delete_reminder(ctx, id):
# code that's the current delete_reminder command```

static canyon
#

But again no clue if it'll work how it's supposed to

gritty wind
#

Yes that's what I suggested, and it should in theory work

#

Otherwise you'd end up with something like

        for id_ in ids:
            try:
                lock(LOCK_NAMESPACE, id_, raise_error=True)()
                ...
            except LockedResourceError:
                failed.append(id)
#

Which is mostly fine, the only issue is we're bypassing the lock_arg, which makes it harder to modify in the future

#

It's not meant to be used in this way

static canyon
gritty wind
#

The lock could in theory be modified to work with multiple resources at the same time too

#

It would probably be the same amount of effort

static canyon
gritty wind
#

Since there's also confirmation, I'd actually move the deletion into the util function, and the confirmation into the main function

#

That way we send one confirmation/failure

static canyon
#

Something like this?```py
@lock_arg(LOCK_NAMESPACE, "id_", raise_error=True)
async def delete_reminder(self, ctx: Context, id: int) -> bool:
"""Acquires a lock on id_ and returns True if reminder is deleted, otherwise False"""
if not await self.can_modify(ctx, id):
return False

    await self.bot.api_client.delete(f"bot/reminders/{id_}")
    self.scheduler.cancel(id_)
    return True

@remind_group.command("delete", aliases=("remove", "cancel"))
async def delete_reminder(self, ctx: Context, ids: Greedy[int]) -> None:
    """Delete one of your active reminders."""
    deleted_ids = []
    for id_ in ids:
        try:
            reminder_deleted = self._delete_reminder(ctx, id_)
        except LockedResourceError:
            continue
        else:
            if reminder_deleted:
                deleted_ids.append(str(id_))

    if not deleted_ids:
        return

    embed = discord.Embed(
        description=f"Successfully deleted the following reminder(s): {' '.join(deleted_ids)}",
        colour=discord.Colour.green(),
        title=random.choice(POSITIVE_REPLIES)
    )
    await ctx.send(embed=embed)```
gritty wind
#

I typically dislike when people use else in a try

#

You just put it in the try

static canyon
#

It's just more explicit as to which line you're expecting to cause the error

#

But fair enough

gritty wind
# 
        for id_ in ids:
            try:
                reminder_deleted = self._delete_reminder(ctx, id_)
                deleted_ids.append(str(id_))
            except LockedResourceError:
                continue
#

Yeah that's true, but not always applicable

#

At the very least i'd remove the if, in what scenario are you not expecting it to return

static canyon
#

And I do still need the if, since _delete_reminder() can return False

gritty wind
#

When would it return False

#

Oh I see

static canyon
#

When the user doesn't have permissions to delete that reminder

gritty wind
#

Alright fair enough

#

So I think we should instead of having that return

#

Check if there are any in the deleted list which aren't in IDs

#

and tell the user which ones those are

#

If you store the failed ones instead of the deleted, it becomes easier to do that

# 
for ...
  try:
    delete
  except:
    failed.append(id)

if failed:
  send_failed_message()
else:
  send_confirmation
static canyon
#

That's not the standard behaviour for this though

#

For reminders when you don't have permission to edit it just fails silently

#

!reminder edit duration 1234 1s

stable mountainBOT
#

There does not seem to be anything matching your query.

static canyon
#

Eh, it needs to be a valid id but it'll fail silently

gritty wind
#

I see, that seems like weird behavior but I don't expect you to change it then

static canyon
stable mountainBOT
#

bot/exts/utils/reminders.py lines 441 to 442

if not await self._can_modify(ctx, id_):
    return```
gritty wind
#

Actually

#

What does the can_modify function do

#

It takes context, so I assume that sends the failure message

#

Yeah it does

static canyon
#

Huh, yeah it does

gritty wind
#

We should probably then make that optional behavior, and send them as bulk

static canyon
#

That's an issue though

gritty wind
#

Otherwise you could have the bot spam 200 messages lol

static canyon
#

Yeah exactly

#

So add a send_denial=True kwarg?

gritty wind
#

Yeah sounds good

static canyon
#

Aight

#

Does this seem a good alternative to the send_denial?py deletion_message = f"Successfully deleted the following reminder(s): {' '.join(deleted_ids)}" if len(deleted_ids) != len(ids): deletion_message += " The other reminder(s) belong to other users so you can't delete them."

gritty wind
#

The failures could also be from the lock

#

It might be better to say could not delete the other ones

static canyon
#

Hmm yeah

#

I can store the failed ones

#

And do something like if len(set(ids) - set(failed)) != len(ids)

#

Eh still a misleading error message though

#

I'll just say couldn't delete

#

Alternatively, py deletion_message += " The other reminder(s) could not be deleted as they were either locked or belong to someone else."to be explicit

gritty wind
#

Sounds good

static canyon
#

Is this the correct overflow syntax for stringspy deletion_message += " The other reminder(s) could not be deleted " \ "as they were either locked or belong to someone else."

#

I.e. using the \ to make multi-line

gritty wind
#

Use parans instead

# 
            deletion_message += (
              " The other reminder(s) could not be deleted "
              "as they were either locked or belong to someone else."
            )
vale ibex
#

which will do 4.X

#

IE this is on 4.3.1 in the PR

#

is this intended?

gritty wind
#

So I’m pretty sure the deps in the lock file don’t actually count

#

Since they’re overwritten by the setup

vale ibex
#

~=4.2 is still 4.X though

gritty wind
#

So when you install as a user you’ll just get ~=4.2

#

And you get to pick

vale ibex
#

I'm only mentioning as I'm not sure if there were more breaking changes in 4.3

#

of course downstream projects can pin to 4.2.X

gritty wind
#

The lib itself should be compat with 4.3 as far as I tested, but if they’re introducing breaking changes on feature releases, we might have to get more specific

#

(In the future)

#

I should also add aiodns to the require shouldn’t i

#

Yeah

#

I’ll do that

vale ibex
#

afaik the only brekaing change that would affect us is the removal of using OSError in 4.3

#

it uses ConnectionError instead

#

"us" being downstream projects such as bot, it won't impact the lib

gritty wind
#

We could go to 4.3 directly in bot

vale ibex
#

Yea that's what I'm planning

#

will pin to latest

gritty wind
#

Cool

#

This was the first blocker for dependabot

#

We might get that now

vale ibex
#

Nice

#

Could you approve bot-core#108

dusky shoreBOT
vale ibex
#

will merge it and then make a new PR/beta with the rc2 changes

#

I droped the asyncrediscache changes, so this is just py3.10

gritty wind
#

Yeah

vale ibex
#

after I push the lock file that is lol

gritty wind
#

Wait how are you doing 3.10 without redis

#

Will we just hold it in an unready state?

vale ibex
#

yea, this is still in beta tag

#

will do a beta3 release with redis

gritty wind
#

Could we retarget a beta branch

#

I’m not sure what the timeline on RC 2 is

vale ibex
#

hmmm, yea might block work otherwise

gritty wind
#

Ye

vale ibex
#

retargeted

gritty wind
#

Bot-core is such a well-oiled machine man

#

It just works

vale ibex
#

yea dude

#

just don't look at netlify ci

gritty wind
#

We should merge the site branch

#

Just steal an account

static canyon
#

@gritty windpy api_response = await self.bot.api_client.get(f"bot/reminders/{reminder_id}")how can I catch a 404 on this?

gritty wind
#

Oh you know what, I'll submit it from a dummy account and approve it

vale ibex
#

lol

gritty wind
#

Or pass raise_for_status as False and check the status code

#

Chris did you lock on a windows machine

#

And what version of poetry

static canyon
gritty wind
#

api_response.status is an int

static canyon
#

But api_client.get() returns the dict?

gritty wind
#

Only if it passes, which is why it's easier to catch

#

This is the code for request

# 
async with self.session.request(method.upper(), self._url_for(endpoint), **kwargs) as resp:
    await self.maybe_raise_for_status(resp, raise_for_status)
    return await resp.json()
static canyon
#

Right

#

I'll catch then

gritty wind
#

#10634: Make -P (pdb) option work better with exceptions triggered from events
If this is true, my life will improve 13-fold

static canyon
#

Also, what's your github?

full fractal
#

I think it's better to keep it as is. I don't think people will attempt to edit their reminders without changes much

#

ichard26

static canyon
gritty wind
#

@vale ibex the PR is minor enough I think that it's more than fine with your review

#

I'll write a changelog and push to pypi, feel free to bump

vale ibex
#

Cool cool

gritty wind
#

Apparently the workflow ignores rcs

#

How did it run on the previous release

#

Wat

vale ibex
#

Does it?

static canyon
#

bot#2232 is now live! CC @full fractal since you said you'd like to take a look 😄

dusky shoreBOT
gritty wind
stable mountainBOT
#

.github/workflows/release.yaml lines 3 to 8

on:
  release:
    types: [published]
    tags-ignore:
      - '**dev**'
      - '**rc**'```
gritty wind
#

I do not

#

understand

vale ibex
#

odd

gritty wind
#

I wonder if ** just doesn't match

vale ibex
#

Because bot-core depends on async-rediscache (1.0.0rc2) which doesn't match any versions, version solving failed.

#

wut

gritty wind
#

No repro :O

vale ibex
#

cleared my pypi cache and it worked

#

guess poetry didn't want to look at the live pypi repo to resolve

gritty wind
#

Ah

#

This action is archaic magic

#

It's locked to ubuntu 18, py3.8 setup@v1, and what's __token__ lol

#

Seb has so much wisdom to teach us yet 😔

vale ibex
# 
if ping:
    # Perform a PING to confirm the connection is valid
    await self._client.ping()

  self.valid = True
``` should this be ```py
if ping:
    # Perform a PING to confirm the connection is valid
    self.valid await self._client.ping()
else:
    self.valid = True
```?
#

in RedisSession.connect

gritty wind
#

PING is non-returning, it'll either error or work

vale ibex
# 
    def ping(self, **kwargs) -> ResponseT:
        """
        Ping the Redis server

        For more information see https://redis.io/commands/ping
        """
        return self.execute_command("PING", **kwargs)
#

returns PONG

gritty wind
#

True

vale ibex
#

wait I guess a ConnectionError is raised if it fails?

gritty wind
#

Yes

vale ibex
#

then yea, no point returning

#

since you'd need to handle that anyway

gritty wind
#

aioredis actually had the same implicit behavior when creating the connection pool

#

This is mostly to keep that same sort of behavior

#

I forgot to add the new return value to the changelog

vale ibex
#

which return value?

#

the delete?

gritty wind
#

Oh that too lol

#

No the one from connect

# 
session = RedisCache()
await session.connect()

# can become
session = await RedisCache().connect()
vale ibex
#

ahhh

#

yea it returns self now

gritty wind
#

It's nothing crazy

vale ibex
#

bot-core#110

dusky shoreBOT
gritty wind
#

Changelogged both

#

Umm

vale ibex
#

wut

gritty wind
#

ok

vale ibex
# 
ERROR: Could not install packages due to an OSError: [Errno 2] No such file or directory: '/home/runner/work/bot-core/bot-core/.cache/py-user-base/lib/python3.10/site-packages/packaging-20.9.dist-info'
#

this is new

gritty wind
#

It's amazing how you changed one line and produced 1000 LOCs diff

vale ibex
#

well, rc2 does change a bunch of sub deps

gritty wind
vale ibex
gritty wind
#

Interesting

#

Rerun?

vale ibex
#

rerunning

#

I blame this hassan fella

#

making bad actions

gritty wind
#

Atrocious

vale ibex
#

passed now anyway

gritty wind
#

Btw you don't need to explicitly specify ping, it shhhhhould have a default

#

Merged, tag?

vale ibex
#

so many are unstable

gritty wind
#

Lmfao

vale ibex
#

now to tear apart the bot pr lol

#

shouldn't be much of a change from what I have

gritty wind
#

Yeah so

#

I think you should keep the client

vale ibex
#

yea, don't want to mess with pools

gritty wind
#

Smart man

#

Chris this is unacceptable

#

there are no perma links in the changelog

#

You know, we're almost at 100 PRs on bot-core and with v8 on the corner, it might be nice to put out a dev-announcement to drum up some hype for it

#

But we should probably triage some work first

vale ibex
vale ibex
#

need to port sir-lance and sir-robin at some point too

gritty wind
#

Btw if you start running into typing issues while upgrading

#

Update types-redis

#

I can't find why or how it got installed, and pip can't either

#

But it did seem to need an update

static canyon
#

bot#2233, bot#2232, bot#2089 and bot#2031 are all needing reviews if anyone's free 😄

dusky shoreBOT
gritty wind
#

Interesting, why is my bot trying to connect to prod address

#

for redis

#

Oop, I had removed my config for testing something, all good

#

@vale ibex Oooh we didn't remove the close from bot-core

#

There's session.client.quit() and session.client.close but you don't actually need to do any clean up

#

It was missed in the changelog for RC 1, I'll add it

vale ibex
#

Ah cool, i'll add it to bot-core

#

Also, bugbear added a late binding closure check

#

very cool

gritty wind
#

I think we already got it on bot-core when I bumped aioredis

#

It's good stuff

vale ibex
#

yea

vale ibex
dusky shoreBOT
vale ibex
#

not sure what is up with the PR title lol

gritty wind
#

Haha

#

Btw what's a good way to mark a function as deprecated

#

I added it in the docstring as a regular comment

#

There's things like sphinx syntax, but we don't really use sphinx

#

In java you just @Deprecated, why can't this be java 😔

vale ibex
outer oasis
gritty wind
#

🔨

vale ibex
#

I think the normal thing is to warnings.warn and to have the notice in the docstring

gritty wind
#

I added a warning too, but deprecation warnings are hidden by default

vale ibex
#

raise BaseException whenever it's called

gritty wind
gritty wind
brazen charm
#

pycharm should be smart enough to understand the warning at least

brazen charm
outer oasis
gritty wind
#

I use my python to autogenerate my java

outer oasis
#

I wish I had been smart enough to generate that crap

#

I have used regex to generate Python

gritty wind
#

And you said java was a bad language

outer oasis
#

It is!

vale ibex
#

Alright, sorted bot#2229

dusky shoreBOT
vale ibex
# gritty wind ```py session = RedisCache() await session.connect() # can become session = awa... 
async def _create_redis_session() -> RedisSession:
    """Create and connect to a redis session."""
    redis_session = RedisSession(
        host=constants.Redis.host,
        port=constants.Redis.port,
        password=constants.Redis.password,
        max_connections=20,
        use_fakeredis=constants.Redis.use_fakeredis,
        global_namespace="bot",
    )
    try:
        await redis_session.connect()
    except RedisError as e:
        raise StartupError(e)
    return redis_session
``` vs ```py
async def _create_redis_session() -> RedisSession:
    """Create and connect to a redis session."""
    try:
        return await RedisSession(
            host=constants.Redis.host,
            port=constants.Redis.port,
            password=constants.Redis.password,
            max_connections=20,
            use_fakeredis=constants.Redis.use_fakeredis,
            global_namespace="bot",
        ).connect()
    except RedisError as e:
        raise StartupError(e)
#

not sure which I prefeer

#

ofc we could do ```py
async def _create_redis_session() -> RedisSession:
"""Create and connect to a redis session."""
redis_session = RedisSession(
host=constants.Redis.host,
port=constants.Redis.port,
password=constants.Redis.password,
max_connections=20,
use_fakeredis=constants.Redis.use_fakeredis,
global_namespace="bot",
)
try:
return await redis_session.connect()
except RedisError as e:
raise StartupError(e)

gritty wind
#

I like numero tres

vale ibex
#

same

gritty wind
#

I ran mypy to try and find any redis issues

#

Bad idea

vale ibex
#

lol yea

#

we already have many

#

I looked for any imports of aioredis / non RedisSession imports from async redis cache

gritty wind
#

Oh we should mark async_rediscache as typed

vale ibex
#

how?

gritty wind
#

It can be added to the package metadata

#

py.typed

vale ibex
#

ohh, it's a lib change?

gritty wind
#

Yeah

#

I'll just push it to main I guess

vale ibex
#

Never touched packaging properly

gritty wind
#

I didn't realize there was a special flag for it

#

Seems only mypy reads it

#

Oh it's a... file?

#

I think

vale ibex
#

SO async-rediscache would be covered by the first point right?

#

since it has in-line annotations

gritty wind
#

Yeah

vale ibex
#

Cool cool

#

There's a bunch of other issues with mypy on bot

#

was ~1k last time I checked

gritty wind
#

Oh then I guess you'll be glad to hear we're down to 200 lol

vale ibex
#

oh lol

gritty wind
#

Well, I only checked the bot directory

vale ibex
#

guess bumping versions has solved a load

gritty wind
#

There's tests too

#

Only 60 there

#

Alright, I pushed typing, we'll fit into some release at some point

#

With that and types-redis, we get 0 mypy warnings from redis, and I'm happy with that

vale ibex
#

nice

gritty wind
#

How do you trigger a compose rebuild from docker desktop

vale ibex
#

I always just use the cli

gritty wind
#

Fair enough

#

We should probably rip out the noqas N802

#

They are all over the test suite, and they're from pep-naming for the builtin methods

#

But I think they have support out of the box now

vale ibex
#

Some do

# 
tests/base.py:35:10: N802 function name 'assertNotLogs' should be lowercase
tests/base.py:85:10: N802 function name 'assertHasPermissionsCheck' should be lowercase
```these still fail
gritty wind
#

They aren't built in methods, so they are properly incorrect

#

But yeah if did want to remove it, we could keep it on those

vale ibex
#

Yea, they're incorrect, but consistent :P

gritty wind
#

True

vale ibex
#

I'll remove the others and leave these

gritty wind
#

It's funny, pycharm doesn't pick up N802

#

So it'll complain about these

#

I've gotten a traceback, but can't quite figure out from what

#

It stems from the error handler

# 
Stack (most recent call last):
  ... discord.py and asyncio stuff
  File "C:\Users\hassa\AppData\Local\pypoetry\Cache\virtualenvs\bot-QP3Q7ay1-py3.10\lib\site-packages\discord\client.py", line 456, in _run_event
    await coro(*args, **kwargs)
  File "C:\GitHub\PyDis\bot\bot\exts\backend\error_handler.py", line 74, in on_command_error
    await self.handle_user_input_error(ctx, e)
  File "C:\GitHub\PyDis\bot\bot\exts\backend\error_handler.py", line 276, in handle_user_input_error
    await self.send_command_help(ctx)
  File "C:\GitHub\PyDis\bot\bot\exts\backend\error_handler.py", line 105, in send_command_help
    await ctx.send_help(ctx.command)
  File "C:\Users\hassa\AppData\Local\pypoetry\Cache\virtualenvs\bot-QP3Q7ay1-py3.10\lib\site-packages\discord\ext\commands\context.py", line 541, in send_help
    return await injected(entity)
  File "C:\Users\hassa\AppData\Local\pypoetry\Cache\virtualenvs\bot-QP3Q7ay1-py3.10\lib\site-packages\discord\ext\commands\core.py", line 182, in wrapped
    ret = await coro(*args, **kwargs)
  File "C:\GitHub\PyDis\bot\bot\exts\info\help.py", line 321, in send_command_help
    await wait_for_deletion(message, (self.context.author.id,))
  File "C:\GitHub\PyDis\bot\bot\utils\messages.py", line 84, in wait_for_deletion
    await message.add_reaction(emoji)
  File "C:\Users\hassa\AppData\Local\pypoetry\Cache\virtualenvs\bot-QP3Q7ay1-py3.10\lib\site-packages\discord\message.py", line 1061, in add_reaction
    await self._state.http.add_reaction(self.channel.id, self.id, emoji)
  File "C:\Users\hassa\AppData\Local\pypoetry\Cache\virtualenvs\bot-QP3Q7ay1-py3.10\lib\site-packages\discord\http.py", line 475, in request
    _log.warning(fmt, retry_after, bucket, stack_info=True)
#

I have the trashcan btw, it got added

#

I think I just got ratelimited, did that always produce tracebacks