#dev-contrib

1 messages · Page 3 of 1

static canyon
#

That's PyCharm expanding everything

#

You've not even got the Optional yet lemon_wink

fossil veldt
#

hm? it's there, Union[..., None]

static canyon
#

Oh yeah

#

PyCharm Unions it, fair enough

fossil veldt
#

how did...

stable mountainBOT
#

bot/exts/moderation/infraction/infractions.py line 431

is_temporary = kwargs.get("expires_at") is not None```
fossil veldt
#

why is there a kwargs.get instead of it just being a keyword argument catderp

static canyon
#

Just makes calling the function easier I guess

fossil veldt
#

yeah I'll just leave it like that

static canyon
#

It is admittedly a bit weird, but yeah, I'd leave it

fossil veldt
#

catderp

=========================== short test summary info ============================
FAILED tests/bot/exts/moderation/infraction/test_infractions.py::VoiceMuteTests::test_voice_mute_user_left_guild
FAILED tests/bot/exts/moderation/infraction/test_infractions.py::CleanBanTests::test_cleanban_doesnt_purge_messages_if_clean_cog_available
FAILED tests/bot/exts/moderation/infraction/test_infractions.py::CleanBanTests::test_cleanban_falls_back_to_native_purge_without_clean_cog
FAILED tests/bot/exts/moderation/infraction/test_utils.py::TestPostInfraction::test_first_fail_second_success_user_post_infraction
FAILED tests/bot/exts/moderation/infraction/test_infractions.py::VoiceMuteTests::test_permanent_voice_mute
FAILED tests/bot/exts/moderation/infraction/test_utils.py::TestPostInfraction::test_normal_post_infraction
FAILED tests/bot/exts/moderation/infraction/test_infractions.py::VoiceMuteTests::test_temporary_voice_mute
7 failed, 457 passed, 1 skipped, 14 warnings in 56.14s
static canyon
#

Yeah, you'll have to rewrite tests

fossil veldt
#

oh right yeah

E           Expected: mock(<MockContext spec_set='Context' id='140294470527200'>, <MockMember spec_set='Member' id='140294470525136'>, 'foobar', expires_at=None)
E           Actual: mock(<MockContext spec_set='Context' id='140294470527200'>, <MockMember spec_set='Member' id='140294470525136'>, 'foobar', duration_or_expiry=None)
static canyon
#

Or rather, update them

fossil veldt
#

so this is an issue

    @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar")
    async def test_first_fail_second_success_user_post_infraction(self, post_user_mock):
        """Should post the user if they don't exist, POST infraction again, and return the response if successful."""
        payload = {
            "actor": self.ctx.author.id,
            "hidden": False,
            "reason": "Test reason",
            "type": "mute",
            "user": self.user.id,
            "active": True,
            "dm_sent": False
        }

        self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"]

        actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason")
        self.assertEqual(actual, "foo")
        self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2)
        post_user_mock.assert_awaited_once_with(self.ctx, self.user)
#

I can't assert the expected static payload because the new time entry is dynamic

#

should I patch the datetime.now function? Or pithink

static canyon
#

Hmm not sure what the correct approach is here

fossil veldt
#

I patched it

static canyon
#

I'm not overly familiar with the workings of the testing library

fossil veldt
#

¯_(ツ)_/¯

static canyon
#

But patching should suffice

fossil veldt
#

will see what happens catlost

static canyon
#

It failed lol

fossil veldt
#

yeah the patched ones didn't work pithink

#

what

TypeError: can't set attributes of built-in/extension type 'datetime.datetime'
static canyon
#

Yeah, you can't patch that apparently

#

I wonder if there's a env var or something that gets set which files can use to know tests are running

#

Probably not the correct approach anyway though

fossil veldt
static canyon
#

The answer could be to remove the test but that's not ideal

fossil veldt
#

like @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime)

gritty wind
#

Don’t patch it

#

Instead separate the assert into an assert_awaited (without the with), and compare the call

#

One of the assert methods will check that the arguments provided match (rather than both dicts being the same)

#

Can’t find it, it might not be in unittest

#

But I’d still go with a similar approach, just remove that argument from the dict

static canyon
#

!help ban

stable mountainBOT
#
Command Help

!ban <user> [duration=None] [reason=None]
You cannot run this command.

*Permanently ban a user for the given reason and stop watching them with Big Brother.

If duration is specified, it temporarily bans that user for the given duration.*

static canyon
#

I'd also be inclined to update all the command param to duration_or_expiry too, making it more obvious you can supply an end date as opposed to relative (nb: also have to update docstring)

#

Up to you I guess @fossil veldt

fossil veldt
#

I'm just trying to find where it is

static canyon
#

It'll be in the infractions.py

#

All of the @command

fossil veldt
#

is it just the docstring?

static canyon
#

Nope, it read the actual parameters

#

But the docstring (of the ban at least) references the duration name

#

So if we change the parameter, you also need to change the docstring if it references the parameter

#

A function's parameters are the bit on a "codeblock", and the description underneath in italics is the docstring

fossil veldt
static canyon
#

So "...bans that user for the given duration or until specified timestamp" or something

fossil veldt
#

this is the current tempban docstring

"""
Temporarily ban a user for the given reason and duration.

A unit of time should be appended to the duration.
Units (∗case-sensitive):
\u2003`y` - years
\u2003`m` - months∗
\u2003`w` - weeks
\u2003`d` - days
\u2003`h` - hours
\u2003`M` - minutes∗
\u2003`s` - seconds

Alternatively, an ISO 8601 timestamp can be provided for the duration.
"""
static canyon
#

!src ban

stable mountainBOT
#
Command: ban

Permanently ban a user for the given reason and stop watching them with Big Brother.

Source Code

Go to GitHub

bot/exts/moderation/infraction/infractions.py:83
fossil veldt
#

also what is that ∗? it's not the ascii *? pithink

static canyon
fossil veldt
#

!help tban

stable mountainBOT
#
Command Help

!tempban <user> <duration> [reason=None]
Can also use: tban

You cannot run this command.

*Temporarily ban a user for the given reason and duration.

A unit of time should be appended to the duration. Units (∗case-sensitive):
 y - years
 m - months∗
 w - weeks
 d - days
 h - hours
 M - minutes∗
 s - seconds

Alternatively, an ISO 8601 timestamp can be provided for the duration.*

fossil veldt
#

that one is fine I guess?

#

just change duration parameter?

#

and then ban would need the docstring update

static canyon
#

Yeah, I think that's good

#

Since it does mention that a timestamp can be mentioned

#

Just need to update the parameter, yea

fossil veldt
# 
"""
Permanently ban a user for the given reason and stop watching them with Big Brother.

If a duration or ISO 8601 timestamp is specified, it temporarily bans that user for
the given duration.
"""
static canyon
#

Not sure how I feel about "for the given duration" given a timestamp isn't a duration

#

It doesn't overly matter though, and I can't really think of anything better

#

And since we're editing this anyway, I'd put reason in backquotes to indicate that it's a parameter

#

I.e. py """ ...for the given `reason` and stop... """

fossil veldt
#

I could just copy the bottom part of the tempban, though...

static canyon
#

Yeah, I think that works

fossil veldt
#

I mean it's the permanent ban command anyways...?

#

why do we have 2 commands again 😂

static canyon
#

"If a duration is specified, it temporarily bans the `user` for the given duration. Alternatively, an ISO 8601 timestamp..."

#

Oh yeah, backquotes around user too

static canyon
# fossil veldt why do we have 2 commands again 😂

One permanent and one temporary. But because the mods are lazy (myself included when I was a mod), we often use the permanent version when we want it temporary to save ourselves 4 (1 if use alias) characters

fossil veldt
#

so

"""
Permanently ban a `user` for the given reason and stop watching them with Big Brother.

If a duration is specified, it temporarily bans the `user` for the given duration.
Alternatively, an ISO 8601 timestamp can be provided for `duration_or_expiry`.
"""
static canyon
#

So the permanent version also allows passing a duration/expiry, and it all gets handled in apply_ban / apply_mute / etc.

static canyon
#

Wait, actually

#

"Alternatively, an ISO 8601 timestamp representing the expiry time can be provided..."

#

Just to be super explit

fossil veldt
# 
"""
Permanently ban a `user` for the given reason and stop watching them with Big Brother.

If a duration is specified, it temporarily bans the `user` for the given duration.
Alternatively, an ISO 8601 timestamp representing the expiry time can be provided 
for `duration_or_expiry`.
"""
static canyon
fossil veldt
#

is it not backquoted? firT

static canyon
#

Sorry, reason

fossil veldt
#

ohhh

static canyon
#

Also have you got Alternatively on a new line?

#

I think same line works better

#

Oh nvm, it'll be too long

surreal veldt
#

Where in Sir Lancebot's code is the part where others can react to bookmark messages and get it to done? I can't seem to find it

vale ibex
stable mountainBOT
#

bot/exts/utilities/bookmark.py lines 143 to 153

while True:
    try:
        _, user = await self.bot.wait_for("reaction_add", timeout=TIMEOUT, check=event_check)
    except asyncio.TimeoutError:
        log.debug("Timed out waiting for a reaction")
        break
    log.trace(f"{user} has successfully bookmarked from a reaction, attempting to DM them.")
    await self.action_bookmark(ctx.channel, user, target_message, title)
    bookmarked_users.append(user.id)

await reaction_message.delete()```
surreal veldt
#

ahh okay thanks

surreal veldt
gritty wind
#

I love it when other people do my work for me

#

Thanks Chris

vale ibex
#

8 minutes+ to rebase

#

I wonder if we can self host so that we have our own job queue

gritty wind
#

I think it’s throttled because GitHub is large

vale ibex
#

Yea

#

if we self hosted we could limit to pydis repos :P

gritty wind
#

Use one of the cups

fossil veldt
#

is it possible to test bot with a site PR that's not merged yet?

#

specifically site#751

dusky shoreBOT
gritty wind
#

Yes check out and build the site image

#

Change the docker compose on bot to use the local image instead of the remote GHCR one

fallen patrol
#

there's the added thing that they bump the pyproject.toml which means when checking they have to lock each and every update individually which means the cache likely isn't possible and that's the major slowdown

fossil veldt
#

@static canyon PR at bot#2234 if you'd like to take a look 😅

dusky shoreBOT
surreal veldt
tawdry vapor
#

I suggest you post your question as a comment on the issue on GitHub.

#

If no one responds for a while then you can link it here

#

But right now it's not clear what you need help with

surreal veldt
#

Can numba be added to the docs?

vale ibex
#

Added :)

surreal veldt
#

thanks!

static canyon
#

Busy today but will take a proper look when I can

static canyon
stable mountainBOT
#
Aye aye, cap'n!

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

ID: 4895
thorny obsidian
#

@gritty wind I think the issue is "inactive" will still show as if the infraction applied to a person was valid, it has just since expired.

Void just means to keep it in the database, but if you query a user's infractions, it won't show up.

gritty wind
#

The thing is I had considered that, but that’s not a thing I think matters. For mods, I’d still want it to be visible because it does no harm when it exists, and I think it can just be exploited. For users it doesn’t matter either way because they can only see a count. One infraction more or less makes no difference when they can also have 10 notes and it’ll all look the same

#

I also think void would be the wrong term in this case

thorny obsidian
#

Void is the correct term though. It means "mark invalid, but still keep"
Currently, if there's a mistaken infraction, mods will have to pardon it and then add a note or amend the reasoning, or ask Joe to delete it from the DB.

The first option will still bias the infraction count and if this happens to a reformed user, it could skew the perspective.

gritty wind
#

Historically we’ve turned down almost every single request to remove from DB btw

thorny obsidian
#

I do remember some instances where it was removed, but okay

gritty wind
#

And if we’re at a point where we’re reviewing by infraction count that’s a larger issue

gritty wind
#

It wasn’t deemed worthwhile

thorny obsidian
#

I just... I'm not sure why you're so against adding in a way to make sure that our infractions are correct and there's a process to mark incorrect ones as invalid, instead of adding in a reason that you then need to trawl through notes and reasoning for

gritty wind
#

I’m not against adding a way to explicitly mark invalids, I am against a way to make them not visible

#

That is a system which requires oversight

#

Something which historically we’ve never required

thorny obsidian
gritty wind
#

How do you programmatically separate when a mod grabs the wrong ID, and when it’s an explicitly exploitative action

#

But yes

thorny obsidian
#

I'm not sure how it would be an exploitative action to mark an infraction as not valid if it's a mod command. Would you then be concerned about the ban command and the reasoning adjustments also being exploited?

gritty wind
#

This is the entire principle of zero trust. Don’t provide a way for things to be exploited even if you trust your entire team. The way it can be exploited is by hiding actual infractions from the moderation team, say by a compromised account or malicious actor. Reasoning adjustments for something like a ban are less likely to be exploited in this way because if I see a positive ban reason I’m not likely to exactly believe that

#

When I see an infrac say please ignore, I check the audit log for it’s history

#

But if you can’t see the actual infraction at all, that’s when the issue arises

#

If we want, we could remove it from the count, but I don’t think that’s necessary. I still want it to be visible to mods

timid sentinel
#

We could provide a separate command to view voided infractions for a user

wild prism
#

keeping a record of them doesnt necessarily mean returning them on an infraction search

#

yeah what wookie said ^

gritty wind
#

I don’t think mods will use it though

#

Why look it up if you don’t suspect foul play already

#

At which point just check the infraction audit log

wild prism
#

i dont think checking the providence of voided infractions should be a normal part of a mods job anyway

timid sentinel
#

Tbh I'm really not worried about that. I can't see how it would practically be exploited

#

Like, if there's a user that repeatedly broke our rules we'd notice soon enough if their infractions went missing

gritty wind
#

Normal mod: !note Wookie Is a known troll

Compromised account: !hide Wookie is a known troll

#

This can be run in a mod spam channel

timid sentinel
#

Yes

gritty wind
#

Be noticed by no one

#

And hide an actual infraction

thorny obsidian
#

But wouldn't you have the same concern over the !ban command then if the account is compromised? Shouldn't we be concerned about all the other mod commands?

timid sentinel
cold island
#

We can show that there are X voided infractions below the summary of infractions in the user embed, and not show it to the user in #bot-commands. That being said I still have the opinion that there are very very specific situations where we'll want to void infractions, and this isn't something we should normally do, so I'm a bit eh on building an entire system around it

gritty wind
gritty wind
gritty wind
#

Wrong Joe

cold island
#

lol

wild prism
thorny obsidian
gritty wind
#

I didn’t deny a feature though

#

I just asked they still be shown

#

I don’t know what the issue with that is

cold island
wild prism
#

wait so what are you saying?

thorny obsidian
gritty wind
cold island
gritty wind
#

Hardly saying no to the feature, especially with what I said in the discussion above

wild prism
#

If the infraction can still be reached somehow
wdym?

cold island
#

But I'd like a warning explaining when to void infractions before confirming the action

timid sentinel
#

I don't feel massively strongly about the issue but I disagree wholeheartedly with the idea of shutting it down because of "zero trust". The exploit has a very low chance of happening and in practice I think the impact would also be very low.

thorny obsidian
wild prism
#

we could even build in a requirement that the infraction is <1hr old or something

gritty wind
gritty wind
#

Your issue with the count seems to entirely be based on moderators moderating entirely on a count

#

Which is a larger issue if we have it

wild prism
#

its not just the infraction message or the count, it clutters the infraction search

thorny obsidian
gritty wind
gritty wind
#

Zig’s solution is a middle ground, though I prefer the full solution

#

Full is not the right term, my proposed solution

timid sentinel
gritty wind
#

Isn’t what

cold island
gritty wind
sharp charm
gritty wind
#

I pinged you like this a couple weeks ago didn’t I

sharp charm
#

ye

cold island
# gritty wind This

Since an infraction search is usually coupled with !u I think showing that the user has voided infracs there in mod channels is ok. And have a field yeah

gritty wind
#

That is something I’m fine with, I just want it to be noticeable

cold island
#

yeah I agree

gritty wind
#

If a user has 10 voided infracs, that’ll cause further investigation so it resolves what I had issue with

#

Most users would probably have 0 based on the proposed use cases

#

(Accidentally using the wrong ID)

timid sentinel
#

Are you saying to show the voided infractions in full or just the number of voided infractions?

cold island
#

That being said I don't look at it from a security perspective. As long as the infrac isn't deleted this isn't a critical exploit, and a very unlikely scenario even in the unlikely event a mod gets compromised. It would require the attacker to be familiar with our commands and setup, and risk getting caught when they can do so much more with the account. Rather, I want it because infractions are records of interactions between the mod team and server members, whether they were made erroneously or not, and should still be visible somehow, and because everyone makes mistakes sometimes.

cold island
gritty wind
#

My ideal solution is to show it in full, the idea being that voided infractions would be rare enough that they always require investigation. Having it shown by default makes it so mods have it right there instead of looking for another command

#

The middle ground would just be showing the count

gritty wind
timid sentinel
#

If we were to show them in full I don't think it's worth implementing given you can mention it's invalid in the reason

gritty wind
#

Look at it this way, it’s easier to implement stricter security when some things are up to standard, than trying to make everything secure from being insecure

#

And I think as we grow as an org and get more mods, and get more likely compromises and malicious actors, it’s something we should consider if we can

hoary haven
#

i would like a void count visible to mods, and not visible to users when they run !user

undecided on whether we show the full details in infraction search or not

gritty wind
#

I don’t think that’s the case

gritty wind
timid sentinel
gritty wind
#

Like as a mod you might go over hundreds of infractions

#

You might see every few thousand one which falls under the proposed voiding criteria

cold island
#

He's saying it's not worth the implementation effort if we show everything anyway

gritty wind
#

I really don’t think that’s an issue of my implementation

#

I think this whole thing was blown out of proportion

#

We’re acting like mods read hundreds of infractions which we’d mark as void

timid sentinel
#

Yeah I didn't mean that as an argument against your implementation

gritty wind
#

(So far the only proposed use case is when we copy a wrong ID)

timid sentinel
#

Just in general if we decided we wanted to show it in full that probably means it's not worth implementing

gritty wind
#

Maybe, I think people get pretty serious about the counts, though I’ve never considered it as the only factor when moderating

cold island
#

No one does, it does subconsciously bias you when it's the first thing you see though

hoary haven
gritty wind
#

I’d agree if it was more than like 1 invalid infraction every 10000 users

#

Seeing a number go from 1 to 2 is not enough to seriously influence, and 10 to 11 is a drop in an ocean

timid sentinel
#

Ah I see what you mean (counts in user embed), sorry I thought zig meant something else

hoary haven
#

i'd much rather see a void infraction count of 1 and ban & warning count of 0, than a ban count of 1, 100% of the time. i think this is a worthy issue to pursue and it has come up a couple times, but i agree with Zig it's usually overshadowed by more pressing things

so we still have this bot issue open for now, i think: bot#240

dusky shoreBOT
hoary haven
#

besides grabbing the wrong user ID, i think this also benefits the few cases where a moderator genuinely misreads a situation and issues a warning that we'd later like to retract. so i'm all for having a 'void' option

#

i would like to get moderation team feedback on this though, so will probably bring this issue to their attention in mod channels

cold island
hoary haven
#

yeah that's fair. i also think we might want to feed messages to mod-meta whenever an infraction is made void

hoary haven
#

looks at site page and notices extra incorrect punctuation.
who put that there!!
checks
is me

unknown.png
outer oasis
#

Never seen this one before. What exactly is it comparing against?

unknown.png
tawdry vapor
#

I believe it means the cache is out of date

#

Yeah. The bot parses all the inventories for symbols when the cog is loaded. When a symbol's documentation is requested, it actually enqueues doc parsing for all symbols on the same page as the requested symbol.

#

To know which symbols are on the same page, it uses the data it gathered about inventories when the cog loaded, which could have been some time ago.

#

But when it's processing the page, that symbol may not actually exist anymore.

vocal prairie
#

!d r

stable mountainBOT
#
Inventories refreshed
stable mountainBOT
vocal wolf
#

@vale ibex merged site#751

dusky shoreBOT
vocal wolf
#

this PR is up for grabs: site#622

dusky shoreBOT
hoary haven
#

@rapid swallow
The story, all names, characters, and incidents portrayed in this issue and incidents implied are fictitious. No identification with actual persons, moderators (living or deceased), places, buildings, and products is intended or should be inferred.

#

@gritty wind as i understand it the reason for that bot issue is so that we can prevent accidental clearing of messages server-wide if a user ID is not given

austere hornet
#

When I start my Sir Lancebot with the code at the link below and I run the .s command (alias for .sudoku), it's saying that it can't find the Sudoku cog (see the screenshot below). Does anyone know why this is happening?
https://paste.pythondiscord.com/qaziqomeju

unknown.png
vale ibex
#

Where did you put the file?

austere hornet
fossil veldt
#

@vale ibex so um, after site#751 it seems the API still doesn't return the last_applied field on a GET, is there some other change we need to do? pithink

dusky shoreBOT
fossil veldt
stable mountainBOT
#

pydis_site/apps/api/serializers.py lines 175 to 187

model = Infraction
fields = (
    'id',
    'inserted_at',
    'expires_at',
    'active',
    'user',
    'actor',
    'type',
    'reason',
    'hidden',
    'dm_sent'
)```
gritty wind
#

It would need to be changed, but we should get everything ready first

fossil veldt
#

is there a documentation for how to make authenticated GET/POST to apis like /bot/infractions/?

#

like just a raw get for testing, I'm not sure how to structure it and what token I should use pithink

fossil veldt
#

seems to work so far though :3

cold island
stable mountainBOT
#

pydis_site/apps/api/viewsets/bot/infraction.py line 38

## Routes```
molten perch
stable mountainBOT
#

manage.py line 15

"DEFAULT_BOT_API_KEY": "badbot13m0n8f570f942013fc818f234916ca531",```
fossil veldt
molten perch
#

As a Bearer token

fossil veldt
#

ah okay

#

still kind of new to web stuff 😅

molten perch
#

Authorization: Bearer <token>

#

Authorization as in the http header

fossil veldt
#

pithink is this wrong?

gritty wind
#

Yeah that’s a query parameter, not the header

#

You probably won’t be able to easily set the header in browser

#

You can use an http client such as postman to set the headers

#

And it obviously allows you to do other things like change the method

#

I personally use insomnia, good app when it works

molten perch
#

I usually just spin up a project with requests installed 😂

gritty wind
#

The fact it’s not httpx is the worst part

atomic ivy
gritty wind
#

Hehe

molten perch
gritty wind
#

Yeah

#

I like httpx for this stuff because it has a much better client interface

#

So you just set the headers and base once

#

Then all the other requests are clean

#

Requests makes it a pain

molten perch
#

Yeah, makes sense

fossil veldt
#

this didn't work somehow catlost

#

gonna go try just running httpx in python I guess

# 
import requests
from requests.structures import CaseInsensitiveDict

url = "http://localhost:8000/api/bot/infractions/infractions-list"

headers = CaseInsensitiveDict()
headers["Accept"] = "application/json"
headers["Authorization"] = "Bearer badbot13m0n8f570f942013fc818f234916ca531"

resp = requests.get(url, headers=headers)

print(resp.status_code)
print(resp.json())

401
{'detail': 'Authentication credentials were not provided.'}
#

I'm so confused

#

that should be my key as well, I haven't changed the docker compose, it still has this line

    environment:
      BOT_API_KEY: badbot13m0n8f570f942013fc818f234916ca531
gritty wind
#

Oh lol it’s Token not bearer

fossil veldt
molten perch
#

Oh right lol, I even had to modify the code jam mgmt to use token instead of bearer so that I can use the botcore api client, silly me.

atomic ivy
#

!rule 9 7

stable mountainBOT
#
Rules

7. Keep discussions relevant to the channel topic. Each channel's description tells you the topic.

9. Do not offer or ask for paid work of any kind.

austere hornet
outer oasis
vale ibex
#

or just suduku, like in your message

austere hornet
#

Yes, it's named sudoku.py

vale ibex
#

cool, are you running in docker?

austere hornet
#

Yep

vale ibex
#

Have you restarted the container since saving the file?

austere hornet
#

Yep

vale ibex
#

Could you send a screenshot of the folder structure you have?

austere hornet
vale ibex
#

oh, you have it in a new folder?

austere hornet
#

Yeah cuz I have 2 files

vale ibex
#

you need to add an empty __init__.py in that folder

austere hornet
#

Ohhh

vale ibex
#

You will need to change a bit more than that actually, to have a multi-file folder like this

#

since it will try ot load all the files in there

#

look at the snakes folder for an example

#

the init has the setup function, and all other files start with _

#

the _ means they donn't get auto-loaded

austere hornet
#

Ohhh I see

#

So like this?

unknown.png
gritty wind
#

You wouldn’t have an underscore before your cog

austere hornet
#

You mean the file with the cog class?

gritty wind
#

Yeah

austere hornet
#

Gotcha

vale ibex
#

which is the current pattern used elsewhere

austere hornet
#

I'm getting this error now when I try to start my Docker container:

unknown.png
vale ibex
#

seems like a docker issue, haven't seen that one before

#

what does google say about the error?

atomic ivy
#

when in doubt, turn it off and on

austere hornet
#

Well it was off already

austere hornet
vale ibex
#

cool

#

turn the docker daemon off and on again

austere hornet
#

Not sure what a circular import is

vale ibex
#

did you remove the _ from the file with the cog class?

austere hornet
#

Oh yeah I forgot, let me try again

vale ibex
#

did you, or not?

#

if you have the setup function in your init file, then you should have the _

austere hornet
vale ibex
#

add the _ back

#

and you will need to update your import to have the _ too

#

since the name of the file has changed

austere hornet
#

So I would have to put from bot.exts.fun._sudoku import Sudoku in the init file?

vale ibex
#

is that where the class is?

austere hornet
#

Yes

vale ibex
#

your folder is named _sudoku?

austere hornet
#

Oh no, the file _sudoku.py

vale ibex
#

Then you'll need to add the folder to the import path too

austere hornet
#

Oh, so from bot.exts.fun.sudoku._sudoku import Sudoku?

vale ibex
#

is that where your class is?

austere hornet
#

Yeah I think that's right

vale ibex
#

cool, try it and see

austere hornet
#

Yep, that worked, but now I'm getting errors related to my code which I can fix later, thanks for your help!

vale ibex
#

cool

#

hmmm

#

for more info

#

Ah, that's acutally paid

austere hornet
fossil veldt
#

is there a way to "raise" the Bad Argument embed from d.py, with a valid argument?

gritty wind
#

You can just raise it and it’ll behave the same

#

!docs discord.ext.commands.BadArgument

stable mountainBOT
#
discord.ext.commands.BadArgument 

exception discord.ext.commands.BadArgument(message=None, *args)```
Exception raised when a parsing or conversion failure is encountered on an argument to pass into a command.

This inherits from [`UserInputError`](https://discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.ext.commands.UserInputError "discord.ext.commands.UserInputError")
clever wraith
#

When trying to run python -m pre_commit run --all-files to format a PR for pythondiscord/site i get this error, i installed all the hooks with python -m pre_commit install, but i still get this error, anyway i can fix it?😅

unknown.png
gritty wind
#

Try to activate your poetry environment first

#

Also it’s going to be pre-commit not pre_commit

clever wraith
#

well since im in windows i think it has to be with an underscore😅

clever wraith
gritty wind
gritty wind
clever wraith
clever wraith
gritty wind
#

Run poetry shell to activate it

#

And try with a dash instead of an underscore

clever wraith
#

alright

clever wraith
gritty wind
#

Could you post the error

clever wraith
#

it cant find pre_commit or pre-commit

gritty wind
#

Try running poetry install again to make sure your environment is in sync

#

It also couldn’t find flake8 in your previous screenshot

outer oasis
#

Can pre-commit be run with python -m?

gritty wind
#

Yes

outer oasis
#

I thought it was a.... Oh, no, it is just a Python package isn't it

clever wraith
#

yeah lol

gritty wind
#

Yeah

clever wraith
#

im not very skilled with poetry lol

gritty wind
#

Just poetry install

clever wraith
#

ahh, its for installing dependencies

gritty wind
#

Yeah that’s the main purpose of it

clever wraith
#

that goes to the env file? or

gritty wind
#

What goes to the env file?

clever wraith
#

sorry, where do the dependencies get installed to your computer?

gritty wind
#

It creates a virtual environment in your app data folder

#

Packages get installed to that

gritty wind
clever wraith
#

ahhhh ic

#

it worked! thank you❤️

gritty wind
#

Yup yup, feel free to ask if you run into any other issues

#

I’d also suggest running pre-commit install again

clever wraith
#

yep i did actually

gritty wind
#

Nice

clever wraith
#

i did

poetry install
pre-commit install
pre-commit run --all-files
hoary haven
#

@fossil veldt hey i've tested your branch for bot#2240 but the command doesn't work for me. it works for you?

dusky shoreBOT
fossil veldt
#

or the purging doesn't work

hoary haven
#

purging doesn't work
!clean does though

unknown.png
fossil veldt
#

for message ID

#

purge only supported duration / timestamp

hoary haven
#

wait what?

#

908477314717868072 is a user ID

fossil veldt
#

pithink uh

#

lemme try

hoary haven
#

i only ever used user IDs with purge

fossil veldt
#

seems to work? pithink

#

I just did purge <my own id>

hoary haven
#

what's up with that double http haha

#

are you on the test server?

fossil veldt
#

mhm

#

my test bot is on * prefix you can have a try

hoary haven
#

woop i'm all over the place

hoary haven
#

ok we gucci, !purge works fine

timid sentinel
# hoary haven purging doesn't work !clean does though

That happened for me too. I think it was because cleaning across all channels only includes channels which everyone can view, so I had to set the permissions for everyone to view channels by default. Not sure if it's the same for you

hoary haven
#

yes when i was sending messages in a random test channel i made (which bot can view, but not everyone) it didn't work

#

does bot check perms for all these channels?

fossil veldt
#

and it goes into this

# 
if channels == "*":
    channels = {
        channel for channel in ctx.guild.channels + ctx.guild.threads
        if isinstance(channel, (TextChannel, Thread))
        # Assume that non-public channels are not needed to optimize for speed.
        and channel.permissions_for(ctx.guild.default_role).view_channel
    }
timid sentinel
#

Yeah it was the view channel permissions thing at the end which made it not work for me

hoary haven
#

!purge

stable mountainBOT
#
Bad argument

At least one user must be specified.

#
Command Help

!purge [users]... [age=None]
*Clean messages of users from all public channels up to a certain message age (10 minutes by default).

Requires 1 or more users to be specified. For channel-based cleaning, use clean instead.

age can be a duration or an ISO 8601 timestamp.*

hoary haven
#

lovely, thank you @fossil veldt !

fossil veldt
vale ibex
#

@timid sentinel I'm not entirely sure where a comment like that would go

#

Since in every other test we do the same thing (import the file, rather than the cog directly)

#

it is a gotcha though, so I wonder if there is a way we can lint against it

#

also nice one on digging up the root cause, I tried for a bit and then gave up since I cared more about the solution 😅

fossil veldt
vale ibex
#

oh

#

uhhhh I can't think of anything

#

Actually maybe the serialiser?

fossil veldt
vale ibex
#

did I forget that

stable mountainBOT
#

pydis_site/apps/api/serializers.py lines 175 to 187

model = Infraction
fields = (
    'id',
    'inserted_at',
    'expires_at',
    'active',
    'user',
    'actor',
    'type',
    'reason',
    'hidden',
    'dm_sent'
)```
vale ibex
#

yea, that'll be it

fossil veldt
#

do I just need to add last_applied to the end? pithink

vale ibex
#

yup, that should be it

#

probably worth testing locally to be sure

fossil veldt
stable mountainBOT
#

pydis_site/apps/api/viewsets/bot/infraction.py lines 153 to 159

serializer_class = InfractionSerializer
queryset = Infraction.objects.all()
pagination_class = LimitOffsetPaginationExtended
filter_backends = (DjangoFilterBackend, SearchFilter, OrderingFilter)
filter_fields = ('user__id', 'actor__id', 'active', 'hidden', 'type')
search_fields = ('$reason',)
frozen_fields = ('id', 'inserted_at', 'type', 'user', 'actor', 'hidden')```
fossil veldt
#

but I guess that's just for filters? Unless I have to add it as a frozen_field as well?

vale ibex
#

we shouldn't need to alter any of those

#

frozen_fields are fields that can't be editted iirc

#

and we want to edit this field

timid sentinel
#

I can't check right now but assuming the nose plugin was just enabled by default and is what caused it there might be a way of disabling it. Would need some testing and a bit out of scope for that PR anyway though

gritty wind
#

I linked the actual cause, it’s not nose

timid sentinel
#

Ooh

#

Well that would make more sense 😅

#

Wait no that's what I was saying

gritty wind
#

The line you linked is only activated for nose plugins as you've said

#

It doesn't activate for us

#

However, lower in the code it adds that fixture

#

Which is for our style of tests

timid sentinel
#

Other ones are setup_module or something else

gritty wind
#

I mean you have the traceback there 😛

#

I don't know where it's set, but hopping in with a debugger does show the right function

#

If you don't have a debugger, turn the setup sync and run pytest tests\bot\exts\backend\test_error_handler.py --pdb

timid sentinel
#

I've not even run it 😅, I'm on my phone. The traceback you posted doesn't seem to contradict what I said though.

gritty wind
#

It does, it doesn't go into the nose branch

#

You almost had the right line, just a bit too high

timid sentinel
#

setup_module in that call you show is defined a few lines earlier, I assumed from the nose branch rather than the others because of the name setup, just that line wouldn't be shown in the traceback because that bit doesn't error (the actual error happens a few lines later as you say)

gritty wind
#

Right, yeah

#

This should probably be raised as an issue upstream

#

It checks if it's callable

#

Should probably check if it's async too

timid sentinel
#

Yeah

fossil veldt
dusky shoreBOT
fossil veldt
#

but also the deploy failed and I'm not sure why pithink

vale ibex
#

there's a fix in progress site#742

dusky shoreBOT
vale ibex
#

it affects all our projects that use netlify

fossil veldt
#

should I just rerun it later or

vale ibex
#

nnah it can be ignored

#

it's just for deploy previews

timid sentinel
gritty wind
#

When'd we fix the github embeds

static canyon
dusky shoreBOT
gritty wind
#

Nice

subtle canopy
#

What user agent would be correct to use when making a req from sir-lancebot?

vale ibex
#

Does the API ask for a user agent in a specific format to be sent?

subtle canopy
#

Not really, but I'm scraping it so I thought it would be better to add it.

vale ibex
#

which issue is this for?

subtle canopy
#

1 sec

vale ibex
#

I can't see anything on their website that would say they allow webscraping, nor is there anything that says it is disallowed

#

Have you contacted them? Might be a good idea to ask them it they are ok with it, and if they want us to use a particular user agent.

subtle canopy
vale ibex
#

They have a contact form on their website.

#

In the footer of the page there's a "Contact Us" link

subtle canopy
vale ibex
#

Sounds good

jolly parcel
#

I am going to be working on implementing File IO for snexbox and eval command. This would likely require mounting an empty to start, temporary directory/file system and such on each eval. I just wanted to ask, for mounting this directory and other things, I would do this in the EvalResource class, correct?

timid sentinel
#

EvalResource is just the eval endpoint

jolly parcel
cold island
#

I want to add a listener to automatically give the bots read perms when a channel is opened. Where should I stick it?

#

Internal? not sure that fits

gritty wind
#

backend not utils

#

new file

#

Probably something like exts/backend/listeners

cold island
#

Hmm is it backend? It's just a qol thing

gritty wind
#

backend is where all the background stuff goes

cold island
#

Wait I remember we had a listener that added the bots to threads whenever one opened

#

Where did we put it?

gritty wind
#

Off the top of my head, there's going to be one in the mod logs, and probably something in the bump code

cold island
#

@timid sentinel in general I'm wondering whether a warning in tests in worth complicating our code for

timid sentinel
#

Using functions feels more intuitive given that's what they're meant for, and prevents the behaviour of getting confusing warnings if you don't use the action

cold island
#

hmmm it's just an object like any other, not sure why it's a hack. Also not sure what you mean by "that's what they're meant for". It's just that defining a function in the middle of the function, which is just a wrapper for one line with no immediate effect you could have written outside of it feels clumsy. Makes me immediately think "let's replace the function call with the line inside it". Maybe even a partial would be better. I'd also be interested to know why those actions aren't run, and why they can't be closed in that case.

static canyon
#

I'm interested in finishing up bot#2017; may I be assigned to it?

dusky shoreBOT
timid sentinel
# cold island hmmm it's just an object like any other, not sure why it's a hack. Also not sure...

Creating a specific "action" function to be passed to a more general function is a common code pattern throughout async and non async code, (given whether it's async is irrelevant). Just passing a coroutine reads to me like a lazy substitution for that pattern with weird and hidden side effects (only works with async code which makes it a bit esoteric, given the pattern has nothing to do with being async; raises a warning if you don't use the function; prevents you from using the function multiple times; and splits up the standard await func() you expect to see when simply executing a function).

Maybe even a partial would be better
I considered that, although a function felt more explicit, and reads more clearly to me. I don't really mind though.

I'd also be interested to know why those actions aren't run
The function that would have called them is mocked in the tests. it could have been patched with a function that just closes the action, but to me it felt like an issue with the code, rather than the tests.

timid sentinel
static canyon
cold island
timid sentinel
cold island
#

yeah 👍

#

will approve once I have a chance to test it

timid sentinel
#

Thanks

static canyon
#

RE bot#2017

Just realised that since it's not a branch on the python-discord repo, I can't actually edit it. Should I just make a new PR and start over?

dusky shoreBOT
gritty wind
#

It should be editable, it’s an option you’d have to explicitly turn off when making PRs

static canyon
#

I kinda just assumed I wouldn't be able to push, but if you're saying I can I don't know how

gritty wind
#

This is what remotes are for

static canyon
gritty wind
#

I know pycharm will allow you to automatically setup a new remote and check out the branch of a PR with a button click

#

Don’t know about other IDEs

#

Anyway the command is just
git remote add <name> url

static canyon
gritty wind
#

Look at the PR tab and check out from there

static canyon
#

Apparently I already have a remote for that repo from a different PR lemon_sweat

static canyon
gritty wind
#

Most git commands take what’s called a refspec which you’re probably familiar with as just the name, but you can also specify a lot of stuff in it, such as the remote, and what the branch will be called locally

#

You can also just do it directly from pycharm

#

Either through the git tab, or the pull request tab

static canyon
#

I am not at all following

#

This just says no PRs found

unknown.png
gritty wind
#

You’re looking for PRs in the fork

#

Not the main repo

static canyon
#

Yeah, idk how to change that

gritty wind
#

See if the plus allows you to switch to a different repo

static canyon
#

Nvm found it

#

It was the settings button

#

Does this matter?

unknown.png
#

I managed to checkout the branch but got that

static canyon
#

So I've just pushed the changes, and it didn't link correctly. It seems to have pushed changes to a new branch on bot, instead of linking to the already existing PR (the branch links to the PR, but the PR doesn't link to the branch)

#

Not really sure how to make bot#2017 use that branch

dusky shoreBOT
fossil veldt
static canyon
fossil veldt
static canyon
fossil veldt
#

it doesn't have you commits

static canyon
#

That's exactly what I'm saying

fossil veldt
#

Can you actually edit the original branch?

static canyon
#

@gritty wind said I didn't need to (from what I understood) and to use remotes and use the pycharm "pull requests" menu so I did but now this happened

static canyon
static canyon
gritty wind
#

Yes that’s what the error above was

static canyon
gritty wind
#

It tried to set arl as your remote for that branch and failed

fossil veldt
gritty wind
#

So it just made it a local branch

fossil veldt
#

did the original fork owner not enable it?

static canyon
#

So... do I try again or what?

static canyon
fossil veldt
#

apparently the way to check is

gritty wind
#

Try this git push arl <branch>

fossil veldt
#

try to edit one of their files on github web, and if you can't it will say do you want to make a fork

static canyon
#

No clue what that did

fossil veldt
static canyon
gritty wind
#

Try just git push arl <local branch name>

#

So don’t specify origin

static canyon
gritty wind
#

That’s done it

static canyon
#

Seems to have worked as the commits now show up on bot#2017

dusky shoreBOT
gritty wind
#

You can now set the tracking branch locally so you’d just use git push instead of specifying arl

static canyon
#

Yep, thanks

#

Hopefully I'll be good now 👍

static canyon
gritty wind
#

There about 14 different ways lol

static canyon
#

Ah nice lol

gritty wind
#

Do you wanna do it on GitHub, git, or your ide

static canyon
#

Someone didn't follow python's Zen lol

static canyon
gritty wind
#

In the git tab, expand the origin folder, find the branch, and delete

#

Make sure it’s under origin, not local branches

static canyon
#

This look right?

unknown.png
#

Welp it worked, nice

#

Thanks 😄

cold island
#

@timid sentinel in https://github.com/python-discord/bot/pull/2245/commits/4e59178d9cc4e23ed4c7b40a2c672ca61e6c9843 it was previously looking for the first message, but now it's looking for the last. Isn't it breaking something?

Also keep in mind that a message can be broken at any point, so you're not guaranteed to have this part in a single message (while the previous string was guaranteed since it'll always be within the first 2000 characters)

GitHub
Update autoreview system by wookie184 · Pull Request #2245 · python...

Not sure if i'm fully done yet (and not properly tested) so marking as draft, but I don't mind if anyone has any general feedback.

timid sentinel
cold island
timid sentinel
#

Could be revisited at some point but for now I think I'll stick with the original method I used

#

I think the case where it might break is if a helper put for Helper! in their nomination reason and it ended up as part of a second message

#

I could always add \n\n**nominated by** or whatever it is if we want to be more sure

timid sentinel
#

In bot#2245 I have a tasks.loop, and two commands (one to enable and one to disable it). After disabling it i'm getting this error:

bot_1          | Traceback (most recent call last):
bot_1          |   File "/usr/local/lib/python3.9/site-packages/discord/ext/commands/core.py", line 200, in wrapped
bot_1          |     ret = await coro(*args, **kwargs)
bot_1          |   File "/bot/bot/exts/recruitment/talentpool/_cog.py", line 105, in autoreview_enable
bot_1          |     self.autoreview_loop.start()
bot_1          |   File "/usr/local/lib/python3.9/site-packages/discord/ext/tasks/__init__.py", line 392, in start
bot_1          |     raise RuntimeError('Task is already launched and is not completed.')
bot_1          | RuntimeError: Task is already launched and is not completed.

Should I be using loop.cancel() instead of loop.stop() in the disable command, or do I need to do something else? I'm not sure I properly understand the difference in this case

dusky shoreBOT
timid sentinel
#

Also which should be in the cog_unload

#

It might be simpler to just leave the cog running and check if it's enabled when it is triggered

cold island
#

Stop allows the current task to complete

#

Not sure about the error

#

Hmm wait it looks like it's erroring on start

vale ibex
#

in autoreview_disable don't do await self.talentpool_settings.set(AUTOREVIEW_ENABLED_KEY, False) until the loop is actually stopped

cold island
#

So I'm guessing the task didn't complete after the stop

vale ibex
#

Yea

#

the task was still running when it tried to be started again

cold island
#

Does cancel try to stop the inner task after it started? Or just the scheduler?

vale ibex
#

cancel will stop the running task

#

await self.reviewer.maybe_review_user() isn't a long-running task right?

#

it just grabs a nominee and posts the review right?

#

If so, we can porbbaly call task.stop() in async def autoreview_disable(self, ctx: Context) -> None: and wait for task.is_running() to be false before actually setting AUTOREVIEW_ENABLED_KEY to False

#

it just might mean that commadn may take a few seconds to run if a task is currently being ran

#

so we may want to but a 🔁 emoji on the invocation or something

gritty wind
#

Wouldn’t doing it in that order open up a race condition

#

What if we switch setting the keg and stopping the loop

timid sentinel
#

There's a possibility for a couple different API calls (checking current messages, getting nominations from site, posting messages for review, creating thread) so it could take a couple seconds

vale ibex
gritty wind
#

Yeah

vale ibex
#

That's what is causing the error above,

#

if the key is false, but the task is still running, another person can come in and set to to start again

#

so calling task.start() will error

#

as it's still running

gritty wind
#

Ah

vale ibex
#

I'd rather a autoreview start call not work because it's still shutting down, than try to start something that is still running

gritty wind
#

Wouldn’t it still fail if someone tried to start something else while it was stopping if you flip it

#

Right

#

That’s fair

timid sentinel
#

How about a lock on the commands and only set the key once it's finished?

#

or do I not need the lock

vale ibex
#

Isn't the AUTOREVIEW_ENABLED_KEY in essance a lock already?

#

just don't set it to false until it's successfully stopped

#

that way a call to start will fail, as AUTOREVIEW_ENABLED_KEY is still true

timid sentinel
#

You could still have two enable or two disable running at once, would that be an issue?

vale ibex
#

oh yea, having separate locks for both commands seems good

#

thought you were suggesting a shared lock

#

a discord.ext.commands.max_concurrency probably does the job

#

a global currency limit of 1 on each command, with wait set to false

timid sentinel
#

I noticed that stop is not async which seemed strange so I looked into the dpy source code. I believe stop just sets a flag (_stop_next_iteration) and then returns. This flag is only checked each time after the task has finished executing, so if the 1hr loop last ran 10m ago, calling stop would make it run again in 50m like normal, and then stop.

stable mountainBOT
#

discord/ext/tasks/__init__.py lines 238 to 248

try:
    await self.coro(*args, **kwargs)
    self._last_iteration_failed = False
except self._valid_exception:
    self._last_iteration_failed = True
    if not self.reconnect:
        raise
    await asyncio.sleep(backoff.delay())
else:
    if self._stop_next_iteration:
        return```
timid sentinel
#

That leaves cancel, which i'm also not too sure about, as it cancels the scheduling task, but that could also be in the middle of running the reviewer code, and cancelling that arbitrarily could leave it in an inconsistent state (e.g. if the message has been posted but the user hasn't been marked as reviewed in the database yet).

#

So the two solutions i'm seeing are

timid sentinel
#

or

#

run the reviewer code in an asyncio.shield so it can finish running if cancelled (and use loop.cancel)

vale ibex
#

leave the task running and have it check the is enabled flag

#

simplifies the disable/enable logic too

#

I guess the only issue would be that starting the autoreviewer again won't cause the task to start right away

#

it will be whenever the next iteration is

#

We could solve that by calling task.restart I guess?

timid sentinel
vale ibex
#

or just manually call the post review funciton within start again

timid sentinel
timid sentinel
vale ibex
#

I'm guessing maybe_review_user already enforces limits on how often they're posted by the way it's written

#

so probably not terrible to call it more than needed

timid sentinel
vale ibex
#

ah, is that the case if it's called twice a the same time?

#

IE it would be fine if say we called it now, and the task called it in 5 mins or somthing

timid sentinel
#

yeah

vale ibex
#

Cool, yea a lock on that maybe_review_user func seems like it'll work then

#

and we just call it from within the review enable command

#

and just contextlib.supress the resource locked error

#

might need to write a task @error handler too

timid sentinel
vale ibex
#

looks good

static canyon
#

Looks like it was a one-off glitch cause the others are in the correct order

cold island
#

Yeah it happens once in a while

fossil veldt
#

Anyone else having trouble running sir-lancebot?

outer oasis
#

What kind of trouble?

fossil veldt
#

I'm getting this on running the main branch

Traceback (most recent call last):
2022-08-08T16:08:57.711411470Z   File "/usr/local/lib/python3.9/runpy.py", line 197, in _run_module_as_main
2022-08-08T16:08:57.712313179Z     return _run_code(code, main_globals, None,
2022-08-08T16:08:57.712335470Z   File "/usr/local/lib/python3.9/runpy.py", line 87, in _run_code
2022-08-08T16:08:57.712974345Z     exec(code, run_globals)
2022-08-08T16:08:57.712987345Z   File "/bot/bot/__main__.py", line 13, in <module>
2022-08-08T16:08:57.715552095Z     bot.load_extension(ext)
2022-08-08T16:08:57.715914637Z   File "/usr/local/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 732, in load_extension
2022-08-08T16:08:57.717033137Z     self._load_from_module_spec(spec, name)
2022-08-08T16:08:57.717047595Z   File "/usr/local/lib/python3.9/site-packages/discord/ext/commands/bot.py", line 663, in _load_from_module_spec
2022-08-08T16:08:57.718709845Z     raise errors.ExtensionFailed(key, e) from e
2022-08-08T16:08:57.718738512Z discord.ext.commands.errors.ExtensionFailed: Extension 'bot.exts.fun.fun' raised an error: ModuleNotFoundError: No module named 'pyjokes'
#

but I definitely should have pyjokes installed in poetry pithink

#

I running via docker-compose up

outer oasis
#

docker-compose up --build

#

rebuild the container, make sure you're not using cached poetry

atomic ivy
#

package changes require rebuilds

outer oasis
atomic ivy
#

for, uh, dockerized apps?

#

because the image doesn't have the changed package(s)

fossil veldt
#

--build worked btw thanks softE

outer oasis
fossil veldt
#

I thought they built into images?

outer oasis
fossil veldt
#

ah, but if dependencies change I'd have to rebuild the image?

outer oasis
#

Or you're editing locally, and all the other layers are cached, so it just takes a couple seconds to update just the layer that contains the source files

outer oasis
# fossil veldt ah, but if dependencies change I'd have to rebuild the image?

If you change a source file, Docker can tell that it's changed, so it will rebuild the layer that contains the source files.
But Docker isn't using your Poetry environment, it's creating it's own inside the image, so it doesn't have anything to compare against to invalidate the cached layer, so you have to trigger the rebuild manually

fossil veldt
#

btw apparently the notion lancebot env is missing a trashcan emoji so I added it

unknown.png
#

right now it's defaulting to the hardcoded one from here

atomic ivy
#

yep, from bot too

fossil veldt
#

also do we really need stuff like this?

#

can we just assume that all cogs will be present?

patent pivot
#

hmmm

#

cog loading is persistent locally right

#

or just persistent in general tbh

fossil veldt
#

what do you mean by persistent pithink

#

I guess the only thing is that the command could be invoked possibly before the other cog loaded on restarts?

#

I don't know if that's even possible

patent pivot
#

if you unload a cog, and restart the bot, that unload is persisted, or so i recall

fossil veldt
#

oh hm

outer oasis
fossil veldt
#

maybe the bigger issue here is using cogs as static libraries pithink

#

I'm pretty sure these functions should just be in an imported module instead

clever wraith
outer oasis
#

I agree

dusky shoreBOT
patent pivot
#

oop

outer oasis
fossil veldt
#

also these were marked as protected with _

#

but used externally

#

🥴

#

I really want these refactored, but I'm just reviewing aboo's PR for the uwu embed fix, not sure if this is relevant to throw in there

outer oasis
# fossil veldt but used externally

They were originally only used there, but the first PR to update UWU moved the logic to a different file, and didn't rename the method.
There are still a couple of PRs open for UWU, right? Let's just do reviews and get everything merged, and then we can make a "cleanup PR"?

cold island
outer oasis
fossil veldt
#

I can't add suggestions here? catlost

outer oasis
#

Makes sense

#

You can't change something that doesn't exist

#

You'd have to select the line above, and start expanding the suggestion from there

fossil veldt
cold island
#

But check the 4th pin 🙂

outer oasis
cold island
#

Fair enough, I'm just triggered by "cleanup" 😄

outer oasis
#

I'm new to working with other people, so I'm open to suggestions

cold island
#

I don't mind some refactoring, just making sure that it doesn't encompass more than it needs to. Didn't have the full context like I said

vale ibex
#

I always see refractor work to be inherently tied to feature work.

Refactoring something just for the sake of it is wasted effort in itself

#

If multiple cogs are using a single function from one of the cogs, then that function should be in one of the utility modules, simply marking the function as public isn't enough imo

thorny obsidian
#

Zig hears "clean up" and immediately thinks back to our Spring Cleanup PR

vale ibex
#

But a pr to do just that one change is wasted review and testing effort as it doesn't "add" anything. Of course if we ever touch this area of the code again, it can be brought up in review then

cold island
#

That depends on the context, if it's bad then doing some refactoring can make the code clearer and more welcoming to future contributors. But yeah, ideally most of these changes should come as part of the review process of the PRs that caused the issues. For example if PR 1 gets merged, then PR 2 can merge main and do some of the refactoring

#

If you don't want to ask the PR 2 author to do it then you have my blessing to commit to their branch / fork in that case (assuming they gave write access to people who have it upstream)

outer oasis
#

👍
We're all staff, so no forks

fossil veldt
dusky shoreBOT
fossil veldt
#

can I just set it as remote? pithink

outer oasis
outer oasis
outer oasis
# outer oasis It's fine as is

"allow members of the upstream organization to edit" is enabled by default, and Zig's question was only a hypothetical anyways

fossil veldt
#

well uh @wide elm what do you think, cuz it's your PR :p
If you want I can add a commit to your branch to refactor fun_cog._get_text_and_embed and fun_cog._convert_embed into module functions of somewhere like bot/utils/messages.py, and updating their local usages in the Fun cog.

#

Also I felt the issue your PR was addressing was more of an upstream one dealing with the _get_text_and_embed static function anyways, so the fix you proposed might be better suited in that static function anyways. Since this will likely come up again if that static function is ever used somewhere else.

wide elm
#

oh what, I could've just made a branch

wide elm
#

I'll address your comments on the PR too

fossil veldt
#

yeah cuz I mean, first of all it's against convention to use those 'protected' methods like this. And also this dynamic cog library loading makes typing pretty wonky.

#

Right now, if the Fun cog is unloaded, the embed parsing feature of .uwu just fails silently and returns nothing

#

Surely that's not... an expected feature?

wide elm
#

its not :p

#

I'll address your comments, commit that, and you can refactor your changes after that, if you'd like

fossil veldt
#

You can work on the refactoring as well if you'd like. Essentially it's just moving the current calls to the 2 Fun cog functions to their own file, probably one of the bot/utils/

#

and then making _get_text_and_embed also grab reply context / embeds so Uwu doesn't have to worry about it

wide elm
#

sure

#

I can do that

fossil veldt
#

lemme know if anything comes up, I'll be here to review it again after changes as well :3

wide elm
#

sure

#

er, I have to head out now, but I can get to this once I get back - is that alright?

fossil veldt
static canyon
#

@wide elm if it were to get approved, did you want to implement bot#2246 yourself? If not I'm happy to

dusky shoreBOT
wide elm
static canyon
#

Ah no worries. There's no rush to get it done so I'll let you do it :)

#

It's a pretty small PR but if you do need any help feel free to @ me

#

And I'll happily review when you're done

wide elm
#

Sure!

wide elm
wide elm
dusky shoreBOT
wide elm
#

@fossil veldt right, I'm out (again :p) I can get your changes in in like half an hour

brisk brook
#

There's no rush Aboo, do it in the time you have thank you.

vale ibex
#

!d r

stable mountainBOT
#
Inventories refreshed
static canyon
#

When I run the tests for bot#2017 locally, I'm getting 32 of this error that isn't even relevant to the PR:```
_________________________________________________________ ERROR at setup of ErrorHandlerTests.test_error_handler_already_handled _________________________________________________________
[gw1] win32 -- Python 3.9.5 c:\users\tizzy\appdata\local\pypoetry\cache\virtualenvs\bot-6mljcnek-py3.9\scripts\python.exe

bot = <module 'tests.bot.exts.backend.test_error_handler' from 'C:\Users\tizzy\bot\tests\bot\exts\backend\test_error_handler.py'>

def setup(bot: Bot) -> None:
    """Load the ErrorHandler cog."""

  bot.add_cog(ErrorHandler(bot))

E AttributeError: module 'tests.bot.exts.backend.test_error_handler' has no attribute 'add_cog'

bot\exts\backend\error_handler.py:333: AttributeError

dusky shoreBOT
gritty wind
#

Hmm it’s defined on the mock bot explicitly

#

Update your branch first

#

It’s very far behind main

static canyon
#

Every time I try to I get errors

#

Mainly that it's not a tracked branch

#

But when I go to make it tracked it decides that there's no root

gritty wind
#

Isn’t the local “arl”

static canyon
#

Yeah

gritty wind
#

Not one random username

static canyon
#

So I specificy arl/fix-issue-2015 but it doesn't find it

gritty wind
#

What’s the command

static canyon
#

These are the only options

unknown.png
#

I'm using PyCharm's UI to do it

gritty wind
#

Gotcha, not sure what it’ll look like for pycharm

#

Uhh what if you just copy the branch name in and hit update

static canyon
#

Nothing happens

#

It remains untracked (whether I tick the "Set as tracked branch" or not)

gritty wind
#

Alright let’s approach this differently, what was the error when you tried to update the branch up to main

static canyon
gritty wind
#

Why would that be an issue anyway, you can update local branches

#

What are you clicking

static canyon
#

The wrong thing lemon_sweat

gritty wind
#

Oh

static canyon
#

I did git merge main and that seemed to work

#

Made a load of changes

#

Tests seem to be fixed

#

Eh, maybe not

gritty wind
#

Yeah tbh it’s likely not going to work

static canyon
gritty wind
#

That branch is more commits behind main than GitHub can show

static canyon
# 
FAILED tests/bot/exts/test_cogs.py::CommandNameTests::test_names_dont_shadow - ImportError: cannot import name 'interactions' from 'botcore.utils' (c:\users\tizzy\appdata\local\pypoetr...
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_add_channel_adds_channel - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_add_channel_skips_start_with_channels - TypeError: __init__() missing 1 required positional argument: 'loop'  
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_add_channel_starts_loop - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_notifier_private_sends_alert - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_notifier_skips_alert - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_remove_channel_removes_channel - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED ```
# 
tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_remove_channel_skips_stop_with_channels - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceNotifierTests::test_remove_channel_stops_loop - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceCogTests::test_kick_move_to_error - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceCogTests::test_sync_move_to_error - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceCogTests::test_voice_kick - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceCogTests::test_force_voice_sync - TypeError: __init__() missing 1 required positional argument: 'loop'
FAILED tests/bot/exts/moderation/test_silence.py::SilenceCogTests::test_force_voice_sync_no_channel - TypeError: __init__()```
gritty wind
#

I think you need to reinstall deps

static canyon
#

poetry install?

gritty wind
#

That’s likely not going to work, I’m assuming the branch was still on dpy 1

static canyon
#

Right

#

So what do I do?

gritty wind
#

I’d run poetry run pip uninstall discord.py then poetry install

#

Check the installed version to make sure it worked

static canyon
#

I think the branch is on dpy2.0

gritty wind
#

Which zip version is it pointing to

static canyon
#

I think discord.py-45d498c1b76deaf3b394d17ccf56112fa691d160 if that's what you mean

gritty wind
#

I don’t think that’s right, we’re on a newer version

#

The one on main is

#

0eb3d26343969a25ffc43ba72eca42538d2e7e7a

static canyon
#

Right okay

static canyon
# 
PS C:\Users\tizzy\bot> poetry run pip uninstall discord.py
Found existing installation: discord.py 2.0.0a0
Uninstalling discord.py-2.0.0a0:
  Would remove:
    c:\users\tizzy\appdata\local\pypoetry\cache\virtualenvs\bot-6mljcnek-py3.9\lib\site-packages\discord.py-2.0.0a0.dist-info\*
    c:\users\tizzy\appdata\local\pypoetry\cache\virtualenvs\bot-6mljcnek-py3.9\lib\site-packages\discord\*
Proceed (Y/n)? y
  Successfully uninstalled discord.py-2.0.0a0
gritty wind
#

So far so good

static canyon
# 
PS C:\Users\tizzy\bot> poetry install                     
Installing dependencies from lock file

  AttributeError

  module 'virtualenv.create.via_global_ref.builtin.cpython.mac_os' has no attribute 'CPython2macOsArmFramework'

  at ~\AppData\Local\Programs\Python\Python39\lib\importlib\metadata.py:79 in load
       75│         """
       76│         match = self.pattern.match(self.value)
       77│         module = import_module(match.group('module'))
       78│         attrs = filter(None, (match.group('attr') or '').split('.'))
    →  79│         return functools.reduce(getattr, attrs, module)
       80│
       81│     @property
       82│     def module(self):
       83│         match = self.pattern.match(self.value)
gritty wind
#

Uhhhhhh hah

#

Idk we could delete and recreate the virtual env

static canyon
#

According to google

This error occurs because two different and incompatible version of virtualenv are installed.

static canyon
#

Can't really remember how to though

atomic ivy
#

poetry env list

#

copy the env name

gritty wind
#

I’m not sure about the conflicting version thing but there’s a poetry command to delete the env

atomic ivy
#

then poetry env rm env_name

gritty wind
#

I wouldn’t suggest using it though

static canyon
gritty wind
#

It always leaves bits of it remaining

#

I just delete the folder

atomic ivy
gritty wind
#

So it just lives rent free on your disk

gritty wind
#

Works when I’m too lazy to remember commands

static canyon
#

... C:\users\tizzy\appdata\local\pypoetry\cache\virtualenvs\bot-6mljcnek-py3.9\lib\site-packages\pip (python 3.9)
So delete \virtualenvs\bot-6ml...?

gritty wind
#

Yeah

#

If you want to do the command first then delete the folder that works

#

Make sure to deactivate first, I learned that the hard way one time

static canyon
#

Apparently I do have two envs for bot 🤔

unknown.png
gritty wind
#

It happens sometimes when things change a lot and I guess it doesn’t register it as being the same env? Frankly I don’t understand how it works completely

static canyon
gritty wind
#

Just deactivate should do it

atomic ivy
#

the env name is generated from the path of the directory

static canyon
gritty wind
#

Uhhh

#

Run deactivate again

atomic ivy
#

you activated it by doing poetry shell, right?

gritty wind
#

If it fails, it’s deactivated

static canyon
# 
PS C:\Users\tizzy\bot> deactivate
PS C:\Users\tizzy\bot> poetry env list    
bot-6mLjCNeK-py3.9 (Activated)
PS C:\Users\tizzy\bot> deactivate     
PS C:\Users\tizzy\bot> deactivate
PS C:\Users\tizzy\bot>
gritty wind
static canyon
#

I would've activated it like a year ago so no clue lmao

atomic ivy
static canyon
#

Probably whatever the guide on site says to do

gritty wind
#

Is it activated from pycharm?

static canyon
#

I guess

gritty wind
#

Oh that just uses the venv commands to activate

#

Well it’s fine, you don’t need to deactivate

#

Just delete the folder and close and open your terminal

static canyon
gritty wind
#

Yup

#

It’s just taking up space and unused

static canyon
#

Okay, I've restarted the terminal

#

Now poetry install?

#

Or do I need to create a new venv

gritty wind
#

Just poetry install

#

It’ll create one for you

static canyon
#

Yep, it's installing

# 
PS C:\Users\tizzy\bot> poetry install
Creating virtualenv bot-6mLjCNeK-py3.9 in C:\Users\tizzy\AppData\Local\pypoetry\Cache\virtualenvs
Installing dependencies from lock file

...```
gritty wind
#

Awesome

atomic ivy
#

just curious, why not use Docker? pithink

static canyon
# 
Package operations: 98 installs, 0 updates, 0 removals

  • Installing pyparsing (3.0.9)
  • Installing wrapt (1.14.1)
  • Installing async-timeout (4.0.2)
  • Installing deprecated (1.2.13)
  • Installing frozenlist (1.3.0)
  • Installing multidict (6.0.2)
  • Installing packaging (21.3)
  • Installing idna (3.3)
  • Installing aiosignal (1.2.0)
  • Installing atomicwrites (1.4.1)
  • Installing certifi (2022.6.15)
  • Installing colorama (0.4.4)
  • Installing iniconfig (1.1.1)
  • Installing attrs (21.4.0)
  • Installing mccabe (0.6.1)
  • Installing hiredis (2.0.0)
  • Installing pluggy (1.0.0)
  • Installing pycodestyle (2.8.0)
  • Installing charset-normalizer (2.1.0)
  • Installing lupa (1.13)
  • Installing pycparser (2.21)
  • Installing pyflakes (2.4.0)
  • Installing py (1.11.0)
  • Installing redis (4.3.1)
  • Installing six (1.16.0)
  • Installing sortedcontainers (2.4.0)
  • Installing tomli (1.2.3)
  • Installing urllib3 (1.26.10)
  • Installing yarl (1.7.2)
  • Installing aiohttp (3.8.1)
  • Installing aioredis (1.3.1)
  • Installing cffi (1.15.1)
  • Installing filelock (3.7.1)
  • Installing platformdirs (2.5.2)
  • Installing pytest (7.1.1)
  • Installing requests (2.28.1)
  • Installing soupsieve (2.3.2.post1)
  • Installing fakeredis (1.7.5)
  • Installing distlib (0.3.5)
  • Installing snowballstemmer (2.2.0)
  • Installing pyreadline3 (3.4.1)
  • Installing flake8 (4.0.1)
Terminate batch job (Y/N)? y
```this doesn't seem to be everything?
#

E.g. discordpy isn't there

#

Or bot-core

atomic ivy
#

it'll get there

#

that's some of the 98

static canyon
#

So it doesn't display everything it installed?

#

Because that's it

#

I think I shouldn't've terminated

atomic ivy
#

it would but you cancelled the install

static canyon
#

No clue why that came up tho

#

Yeah

#

I assumed that meant it was done lemon_sweat

static canyon
atomic ivy
#

does PyCharm not support running inside the container like VSCode does?

static canyon
#

🤷

#

That fixed it, thanks @gritty wind 👍

# 
FAILED tests/bot/rules/test_mentions.py::TestMentions::test_mentions_exceeding_limit - AttributeError: 'str' object has no attribute 'bot'
FAILED tests/bot/rules/test_mentions.py::TestMentions::test_mentions_within_limit - AttributeError: 'str' object has no attribute 'bot'
FAILED tests/bot/rules/test_mentions.py::TestMentions::test_ignore_bot_mentions - AttributeError: 'str' object has no attribute 'bot'
```now I just need to resolve these
#

The code being resolved.author.bot (where resolved should be a discord.Message)

gritty wind
#

Makes some thing easier

#

Did you run install again after terminating

static canyon
gritty wind
static canyon
#

I imagine I need a mock MessageReference object or something?

gritty wind
#

I’ve gotta run right now but if you’re still stuck we can go over it

static canyon
# 
PS C:\Users\tizzy\bot> poetry run task test tests/bot/rules/test_mentions.py
================================================================================================ test session starts ================================================================================================
platform win32 -- Python 3.9.5, pytest-7.1.1, pluggy-1.0.0
rootdir: C:\Users\tizzy\bot                  
plugins: cov-3.0.0, forked-1.4.0, xdist-2.5.0
gw0 [3] / gw1 [3] / gw2 [3] / gw3 [3] / gw4 [3] / gw5 [3]
...                                                                                                                                                                                                            [100%]


================================================================================================ 3 passed in 10.84s =================================================================================================
# 
PS C:\Users\tizzy\bot> poetry run task test                                 
================================================================================================ test session starts ================================================================================================
platform win32 -- Python 3.9.5, pytest-7.1.1, pluggy-1.0.0
rootdir: C:\Users\tizzy\bot
plugins: cov-3.0.0, forked-1.4.0, xdist-2.5.0
gw0 [465] / gw1 [465] / gw2 [465] / gw3 [465] / gw4 [465] / gw5 [465]
.............................................................................................................................................................................................................. [ 44%]
.....................................................................................................................................s................................................................FF.F.... [ 88%]
.....................................................                                                                                                                                                          [100%]
...
============================================================================================== short test summary info ==============================================================================================
FAILED tests/bot/rules/test_mentions.py::TestMentions::test_ignore_bot_mentions - AttributeError: 'str' object has no attribute 'bot'
FAILED tests/bot/rules/test_mentions.py::TestMentions::test_mentions_exceeding_limit - AttributeError: 'str' object has no attribute 'bot'
FAILED tests/bot/rules/test_mentions.py::TestMentions::test_mentions_within_limit - AttributeError: 'str' object has no attribute 'bot'
#

I'm so confused. If I run tests then the 3 in /rules/test_mentions.py fail, but if I run the tests in that file directly they all pass

#

Just ran tests again without changing any code and now it works??? 🤷py PS C:\Users\tizzy\bot> poetry run task test ================================================================================================================================================================================ test session starts ================================================================================================================================================================================= platform win32 -- Python 3.9.5, pytest-7.1.1, pluggy-1.0.0 rootdir: C:\Users\tizzy\bot plugins: cov-3.0.0, forked-1.4.0, xdist-2.5.0 gw0 [465] / gw1 [465] / gw2 [465] / gw3 [465] / gw4 [465] / gw5 [465] ............................................................................................................................................................................................................................................................................................................................................................................... [ 78%] ...........s...................................................................................... [100%]

static canyon
#

So I've run these tests dozens of times and it seems that poetry run task test tests/bot/rules/test_mentions.py will always work, but poetry run task test will sometimes fail and sometimes not

gritty wind
#

Could you push your updated branch

static canyon
#

Yeah, gimme a sec

static canyon
dusky shoreBOT
static canyon
#

I guess we'll see what happens on the github tests

#

But from locally it sometimes passes and sometimes fails

gritty wind
#

Oh these are tests for your code gotcha

#

You’re still a little out of sync btw but that’s probably unrelated

#

I didn’t realize this was an issue with new code, and I’ll need a PC to properly look over it

#

I can’t get to that till tonight at the earliest

static canyon
#

Yeah no worries

#

For context, the issue was that the tests passed the message author as the string 'bob' instead of a MockMember with the name set to 'bob'.

Prior to this the rule the code didn't use .author.xxx so it didn't matter, but now it does, so I've replaced it with MockMember(name='bob').

#

That then meant the return types didn't match, so I also updated the return types to be the MockMember as well

#

But now for some reason there's still strings somewhere causing the 'str' has no attribute 'bot' on resolved.author.bot

vale ibex
stable mountainBOT
#

setup-python/action.yaml lines 61 to 64

- name: Setup python
  uses: actions/setup-python@v2
  with:
    python-version: ${{ inputs.python_version }}```
gritty wind
#

ID?