dev-contrib

[glass pecan]

who was against that last time

i know i was rather against the import styles, but i'm over that now and think it's fine

[woeful thorn]

It ends up being an insanely massive PR

[glass pecan]

sure but we're not manually formatting at the point of changing to it if we went with it, as we'd just autoformat the project once and then move along

[woeful thorn]

🤷‍♂️

I should have left the branch

[clever wraith]

Are seasonal bot alert really going in #dev-log cuz I slept and still no activity.

[glass pecan]

that was the biggest issue with massive linting changes in the past afaik

we manually updated

which sometimes introduced bugs because we're human

but it's true it does absolutely obliterate blame

[clever wraith]

because we're human
Yeah I doubt that

[woeful thorn]

There you go

Do you believe us now?

[clever wraith]

Nice

[eternal owl]

thanks a lot for the gist @glass pecan

[glass pecan]

np. sorry i didn't provide it earlier

didn't realise you were on that issue before lemon mentioned

[eternal owl]

I will render it in embed to see what has to be manually changed

[glass pecan]

the first gist i gave you is exactly the current embed description content for each one

they all are confirmed to work fine in discord

[eternal owl]

ooh

[glass pecan]

the line issue is only present in github

[eternal owl]

ohk gotcha

[glass pecan]

atm this is what things are like

as a reference i'll use info from ask

• Don't ask to ask your question, just go ahead and tell us your problem. 
• Don't ask if anyone is knowledgeable in some area, filtering serves no purpose.
• Try to solve the problem on your own first, we're not going to write code for you.
• Show us the code you've tried and any errors or unexpected results it's giving.
• Be patient while we're helping you.

this is the raw markdown for the list

it renders in an embed on discord fine because discord don't care about line ending in two spaces

!embed • Don't ask to ask your question, just go ahead and tell us your problem.
• Don't ask if anyone is knowledgeable in some area, filtering serves no purpose.
• Try to solve the problem on your own first, we're not going to write code for you.
• Show us the code you've tried and any errors or unexpected results it's giving.
• Be patient while we're helping you.

[stable mountain]

• Don't ask to ask your question, just go ahead and tell us your problem.
• Don't ask if anyone is knowledgeable in some area, filtering serves no purpose.
• Try to solve the problem on your own first, we're not going to write code for you.
• Show us the code you've tried and any errors or unexpected results it's giving.
• Be patient while we're helping you.

[glass pecan]

so we have a valid list

but in github, the render shows

to fix this you can be sure to add two spaces on each line

• Don't ask to ask your question, just go ahead and tell us your problem.  
• Don't ask if anyone is knowledgeable in some area, filtering serves no purpose.  
• Try to solve the problem on your own first, we're not going to write code for you.  
• Show us the code you've tried and any errors or unexpected results it's giving.  
• Be patient while we're helping you.  

if you highlight the ends of these lines in the codeblock, you'll see the extra whitespace

[eternal owl]

I do

[glass pecan]

and this renders fine in github

[eternal owl]

where are you rendering on github?

[glass pecan]

in the gists

[eternal owl]

the image

first img

https://cdn.discordapp.com/attachments/635950537262759947/681714843686142029/unknown.png

this

[glass pecan]

first image is the first gist link i sent

[eternal owl]

ohk Im looking at the 2nd

[glass pecan]

https://gist.github.com/scragly/9923e98d0b8f497639a7ada1d21d3e21#file-ask-md

please make sure you're looking at the right ones

[eternal owl]

yes

I should use 2nd link

[glass pecan]

the 2nd link is an example

it adds indescriminately the two spaces on every single line

this adds extra whitespace to the codeblocks that should not be there

[eternal owl]

ohk

[glass pecan]

i explained that in my original messages to you

[eternal owl]

I got it now

[woeful thorn]

If you use actual asterisks then GH won’t need the trailing spaces

[glass pecan]

yes, but discord don't render the same list style as it doesn't support markdown lists

[woeful thorn]

I think we should probably have the files as something that renders natively and convert it somewhere

[glass pecan]

why add a conversion step when it's not necessary

[woeful thorn]

I don’t think people are going to search for the correct symbol for our embeds

And “just write markdown” is a lot more attractive to prospective contributors

[glass pecan]

if we add the two spaces to the non-codeblock lines, it'll render perfectly fine in both

!embed • Don't ask to ask your question, just go ahead and tell us your problem.
• Don't ask if anyone is knowledgeable in some area, filtering serves no purpose.
• Try to solve the problem on your own first, we're not going to write code for you.
• Show us the code you've tried and any errors or unexpected results it's giving.
• Be patient while we're helping you.

[stable mountain]

• Don't ask to ask your question, just go ahead and tell us your problem.
• Don't ask if anyone is knowledgeable in some area, filtering serves no purpose.
• Try to solve the problem on your own first, we're not going to write code for you.
• Show us the code you've tried and any errors or unexpected results it's giving.
• Be patient while we're helping you.

[glass pecan]

this is with the spaces

discord strips the trailing ones

actually that might be clientside lol

[eternal owl]

there are spaces in the starting

[glass pecan]

no idea what you mean iceman

if you mean after the bullet point, they're meant to be there

[eternal owl]

nvm

[clever wraith]

How do you add a enviroment variable ?
like one of my upcoming PR needs a env variable . it is an api key

ok i will need someoene to do that for me

[woeful thorn]

To do what for you?

[clever wraith]

add an enviromental variable

[woeful thorn]

How are we supposed to do that for you?

How did you add any of the other environment variables that the bot needs?

Are you asking about your computer or our server?

[clever wraith]

server

my upcoming PR will need it ,
as you can see in that screenshot . Jos added API for .games command

[glass pecan]

"upcoming PR"

[clever wraith]

haha

but that one was added on a issue.

[glass pecan]

don't care, add the requirement as part of your PR description

one of the core devs will see it and add it

[clever wraith]

I thought that i have to do something in pip file at first , but then i saw that comment by jos-b

[woeful thorn]

I was being preemptive because I was in the middle of generating other API keys

Just mention that you need a key in the PR and we’ll make sure to add it before it’s merged

Or we’ll forget and crash the bot

[glass pecan]

again

[woeful thorn]

Either way don’t worry

[clever wraith]

Ok

again ?

[glass pecan]

this has been an issue before due to the requirement not being super clear in a PR description

so make sure you make note of it, and we'll make sure it's present when it needs to be

[clever wraith]

ok what happens if we get done with quouta of api request ? how do you handle that ?
do you send a embed saying we are out of API request try again later ?

[glass pecan]

yes, you'll need to send an informative message saying it's ratelimited or used up

[clever wraith]

ok

[cold moon]

Will this https://github.com/python-discord/bot/issues/718 issue get approved or not?

[crude gyro]

@cold moon that issue is fine to work on. but keep in mind that it's a full stack issue - changes will be required both in the site and on the bot.

[cold moon]

Ok

[cold moon]

Site is up... now is only bot...

[cold moon]

When I try to mute with Python bot, I got problem: Bot don't have permissions, but I gave bot every permission that is possible. What is wrong?

[glass pecan]

likely heirarchy

are you sure the bot's top role is higher than the muted role

[cold moon]

What exactly I have to do in site for !unmute and !unban command reasons?

[hardy gorge]

I think this fits into a larger question of the design of our database table for infractions.

Currently, we don't have a field for such a reason, which means that we'd have to store an additional entry.

However, this is annoying, as it creates another infraction object for that individual while we are actually trying to end one of their infractions.

There are two ways to solve this:

• Have an infraction table and an "actions" table with actions for that infraction.
• Have an additional "end reason" field

The watch channel currently uses the ugly "add a second infraction" method, which kinda sucks

Given that we're now looking to add more additional "end reasons" to other infractions, we should really look into a different table design

Anyway, regardless of the strategy, this is essential functionality, so be sure to communicate about your plans before implementing them. It could lead to quite a discussion and it's better to do that before writing code than after.

[molten bough]

I think the way I'd probably do it is to have a general moderator actions table

[hardy gorge]

yes

[molten bough]

But it might be more difficult to filter infractions on

[hardy gorge]

I think this hooks into a larger question about infractions and how we want to store/handle them. I think a good way to solve this is to have two tables, one with the main infraction object (mapping of type, user, infraction id) and one with actions per infraction (infraction id, Action verbs field [creation, edit, end, and void], reason, datetime). The model for the former table would also have a one-to-many relationship with the model for the second.

It's not particularly difficult in the set-up, but the migration is going to be difficult

[molten bough]

Hmm, yeah

[hardy gorge]

It would solve a lot of the issues we were thinking of (like being able to void an infraction without changing the immutable nature of the infractions table)

[molten bough]

It might be nice to have a way to track non-infraction actions as well though, like purging sets of messages, so you don't have to search the modlog for it

Although I guess you could just have a null relation then

[cold moon]

I think I let this to do for someone other. I'm not really good with Django, so I better don't start with this issue...

On bot side, I can help

[hardy gorge]

That's totally fine.

It's not a priority issue in anyway (just a "nice to have" one), but the implementation is critical since it affects the core of our moderation toolset

[crude gyro]

I like the idea of an unban reason, but I don't really need one urgently.

if it's this complex, we might want a core dev handling it

but we don't have core devs to spare for that right now

so let's leave it for the time being

[eternal owl]

I can try it after the tags thing

[hardy gorge]

From quickly looking over the thing, I think this will require some major changes around the bot (a lot of things use the infraction system) and the site part. That means that we'd need to go through a proper discussion and planning stage first and we're currently already working on some other major projects, like deseasonifying seasonalbot and implementing a new help channel system.

That means we don't have the "human resources" to spare for such an extensive project that is mission critical

So, let's do it later (if we decide that we want it)

[clever wraith]

ahh man i wish seasonalbot logs come back ,
For me I don't wanna know anything happenes in @stable mountain while my PR and issues are only for @dusky shore.
Also i feel like there will be less attention to @dusky shore

[glass pecan]

the logs still exist, and you're more than capable of tracking your own issues via email and github

things won't be changing back, so you can stop trying

[brazen charm]

mail keeps you updated on anything relevant for your prs or anything you commented on

[glass pecan]

there's even a nice button to subscribe to updates on issues you haven't commented on

[clever wraith]

Also i feel like there will be less attention to @dusky shore

[glass pecan]

things won't be changing back, so you can stop trying

[green oriole]

They are now in #dev-log?

[glass pecan]

yep

[green oriole]

Oh yeah

Make sense

[glass pecan]

yep, especially since some of our more active devs were neglecting the old log channel

didn't really make sense to keep it separate

[eternal owl]

@clever wraith ur on linux?

[clever wraith]

yes

[eternal owl]

install mailspring

desktop gui for gmail

[clever wraith]

I have my firefox always open

so no

[eternal owl]

keep that running in the background and you will get notification, should be easier to keep track of github

[clever wraith]

my phone get a notification for mails that get relayed to my watch

so same thing

[glass pecan]

so you shouldn't miss a github notification, excellent

[eternal owl]

okay

[hardy gorge]

I think there's going to be more attention to seasonalbot from the core dev group; the lesser attention to the project otherwise is probably more related to our decision not to do events every two months

We will still have a larger event in October

[green oriole]

_Hacktoberfest hype! _

[eternal owl]

whats been planned?

[hardy gorge]

A Seasonal Bot event instead of a general hacktoberfest for all repositories

We will still do hacktoberfest branding and give it some attention

Last year just proved to be far too much work for the reviewing team and we don't really need a hacktober sprint for our other repos anyway

So, we're really going to focus on introducing people to Open Source with Seasonal Bot instead of having people complete as many PRs as they can org-wide for a t-shirt

[cold moon]

I don't understand what is SeasonalBot future when season get removed.

[brazen charm]

all the features it has unrestricted

[woeful thorn]

Not all of them, no. There will still be time-based restrictions on commands where it makes sense

You can read the description here: https://github.com/python-discord/seasonalbot/pull/329

[brazen charm]

things like aoc and hacktober ofc don't have any use outside of their timeframe

[mellow hare]

Debatable regarding AoC

[hardy gorge]

I don't understand what is SeasonalBot future when season get removed.
@cold moon

Apart from removing the main season manager, Seasonal Bot itself will still exist as a project people can work on to get experience with open source programming. The main change is that we won't have a seasonal event every two months. We are planning a larger Seasonal Bot event in October, so the bot won't go away.

[cold moon]

Ok

[hardy gorge]

It's going to tackle one issue I had with the current set up: A lot of the times, PRs were only merged towards the end or even after the season had ended. That meant/means that those features are not actually available to play with until the next year comes around. We're now going to allow more features to be used the whole year round, which means that people can actually show off the features they've implemented..

[cold moon]

https://github.com/python-discord/bot/issues/767 For this, what is all filters. I know there is attachment and spam filters, but is there more?

[woeful thorn]

They should all be in the antimalware, antispam, and filtering cogs

[cold moon]

ok

I think easies way to check for this is:

1. Making new constant staff_roles to Roles in constants.py that contains admin, moderator and owner roles.
2. Check does at least one of these roles match (staff_roles and message author roles). If yes, return.
   Should this work?

[hardy gorge]

There should already be a staff container constant

https://github.com/python-discord/bot/blob/master/bot/constants.py#L586

[woeful thorn]

Hmm, should we do the grouping in either the YAML or constants.py? There some in both and it's a little confusing

Mark could probably slide that into his overhaul of the constants

[hardy gorge]

I feel like we should have things that need to be changed for dev set-ups (like the actual role IDs) in the YAML; these groupings are of a more fixed nature, so I don't see the value of having them in the YAML.

I haven't thought about it for long, though

[cold moon]

But should I use STAFF_ROLES or MODERATION_ROLES?

[woeful thorn]

staff

[cold moon]

ok

Is this good way to check:

        if len(set(STAFF_ROLES).intersection(set([role.id for role in message.author.roles]))) > 0:
            return

[hardy gorge]

You can also do any(role.id in STAFF_ROLES for role in member.roles)

[cold moon]

Oh, it's more clear. I'll use this

[cold moon]

I added this only to antimalwarecheck, due spamcheck and filtering already have these? Is there no more checks?

And also, I think one thing that maybe should changes:
I see much:

# I used defcon as a example
await ctx.invoke(self.bot.get_command("help"), "defcon")

but this can be simplified

await ctx.send_help("defcon")

[woeful thorn]

Those were all written before the method was (recently, v1.3.0) added, but yes

Migrating to new API features should be its own PR

[cold moon]

But is theses all checks that should ignore staff? Just when they is, I'll make PR

[woeful thorn]

I don't know, I haven't looked at the files

If you have and they're all appropriately whitelisting, then just open the PR

[cold moon]

I'll check all cogs and when I don't find, I'll PR

[cold moon]

Does anyone have currently problems with GitHub pushing? I get HTTP 500 error (Internal Server Error).

[tawdry vapor]

I can push fine but https://www.githubstatus.com/

[molten bough]

yeah, GH is having trouble

I can't save repo settings either

[cold moon]

OK, then I'll have to wait (this commit will be 2 new lines and one new 2 new imports)...

[clever wraith]

i never saw github having issues ever before. maybe it always happens when i am sleeping

[rocky bloom]

idt github often has issues

[cold moon]

Can anyone quick review my PR (It's 2 lines adding + 2 import items adding)?

[woeful thorn]

Seems like you completely reverted the initial commit

[cold moon]

Fking PyCharm... I'll fix this now

[cold moon]

Done

[woeful thorn]

Can you please start fresh on a new branch, this shouldn't need 4 commits

And please remember to lint your code before you commit

A pre-commit configuration is provided with the repository if you don't want to have to remember

Alternatively, you can squash the commits down into one on the current branch

[tawdry vapor]

Is it me or does this still show conflicts? https://github.com/python-discord/bot/pull/750

I pushed a merge like 9 hours ago, but that was during GH having issues

[woeful thorn]

I don't think it got the commits, the last commit was on the 15th

[tawdry vapor]

The commit is in the branch but not the PR

[woeful thorn]

It was rejecting my pushes earlier

Oh

[tawdry vapor]

I'll probably need to push something again to fix it

[woeful thorn]

Try flipping the base

[tawdry vapor]

Good idea

[woeful thorn]

or target

flip the thing

for the stuff

[tawdry vapor]

Thanks, that looks like it's going to work. It's building now

And for my other PR...

Oh ok it finally started a new build

[cold moon]

I merged my commits to one in my Python bot PR (AntiMalware cog staff ignore). Can now anyone review this?

[tawdry vapor]

Yup, I've been requested for the review but it will have to wait until tomorrow. I'll likely get around to it in the morning.

[clever wraith]

Is API high ?

[tawdry vapor]

No. Look it up yourself.the length is correct

[brazen charm]

is this right in the reddit cog?
if self.access_token and self.access_token.expires_at < datetime.utcnow(): revoke
shouldn't it be revoked if it expires after now?

[tawdry vapor]

Maybe it assumes it's already revoked otherwise? Not sure

I think I wrote that but I don't remember my thought process

[brazen charm]

it auto expires in an hour, so it should only check it if it hasn't expired yet right? With the expires_at being higher than current time.
Just making sure before I commit anything

[tawdry vapor]

Sorry I can't confirm this right now. I'm in bed and should be sleeping. Maybe someone else can help. Otherwise ping me in the PR/issue so I remember to get back to you.

[cold moon]

One thing about this Reddit issue: Is better use:

if any(value in (RedditConfig.secret, RedditConfig.client_id) for value in ("", None)):

than defining new variable invalid_values and using this in check. (L294)? Not sure about this, anyone feedback?

[crude gyro]

that line is pretty ugly

too dense.

I don't really know what you're asking beyond that though

[cold moon]

Currently it's

invalid_values = "", None
if any(value in (RedditConfig.secret, RedditConfig.client_id) for value in invalid_values):

One other way to solve this is:

if {RedditConfig.secret, RedditConfig.client_id} & {"", None}:

It's from this PR https://github.com/python-discord/bot/pull/785

[crude gyro]

I do kind of like that set intersection solution, because it's fun and novel

if readability was the goal, I probably would've just tried to solve it in more than one line

is there really that big a risk that either of these will be set to a valid but falsy value? I'm not sure I really see the problem with a simple if RedditConfig.secret and RedditConfig.client_id

sure, it's not as precise, but why would they be set to 0 or [] or whatever. aren't they constants that we control?

[cold moon]

I think this is best way. Just due current check look only for empty string and None, but this do this too

[crude gyro]

yeah. just do that

[eternal owl]

https://github.com/python-discord/bot/pull/783
should I wait for this to be merged before working on the tags issue(Cuz i have to install python3.7)

[woeful thorn]

That’s entirely up to you

[tough imp]

The annoying part about that @cold moon is docker will set unconfigured env vars to empty strings, but running without docker gives None, at least thats my understanding

I think your proposed solution of checking truthiness is fine

[woeful thorn]

I think it’s being over complicated tbh

We need an explicit None check because it raises an exception, everything else is already handled fine

[tough imp]

Yeah I agree now, at the time I did not realize this difference between docker / non-docker

so checking for None made no sense to me

[molten bough]

I think docker only does that if the env vars are in the dockerfile, right?

I noticed that behaviour before and it made me wonder why people even define them unset there

it could actually be intentional

[brazen charm]

Can revert the relevant commits if we only want to check against None as the others then get handled when the auth fails, but if we're already doing a check against the values can loading the cog, waiting to fetch something from reddit and then unloading after 3 unecessary requests to reddit and getting an exception in the log and an error message

They're fetched through the .env file but feels like it just goes with var= if it's not present which is an empty string

[tough imp]

Yeah I think so @molten bough, docker even logs a warning that they will be set to empty strings

[woeful thorn]

You don’t have to revert anything, I just don’t think it’s something that needs to be engineered to death

An interesting conversation, sure, but I think we’ve adequately fixed the problem

[cold moon]

@tough imp but none and empty strings is both false....?

[tough imp]

Yea, I'm just giving context to the confusion around that

[eternal owl]

I just moved to manjaro from ubuntu and saved all my projects in google drive (including bot and site), when I downloaded it, all . files changed to _

like .gitignore changed to _gitignore

[molten bough]

That's why you use Git

[eternal owl]

so I re-clone?

[molten bough]

Yep, re-clone, set up again

[eternal owl]

okay

[molten bough]

The point of git is that you don't need to make manual backups like that when you move OS

[eternal owl]

okay

but why did that change happen btw 🤔

[molten bough]

That's just how mafia google drive works

[eternal owl]

okay

[cold moon]

Should Python bot have !package <name> command for fetching package information from PyPi?

[hardy gorge]

That does sound cool. It would be a nice companion to the pep and docs command. Could you make an issue with a proposal so other people can weigh in?

[cold moon]

ok

[brazen charm]

only problem I can see is how to get the data, as you only have an arbitrarily formated readme

had some trouble with the docs and those all have roughly the same structure

[hardy gorge]

That's true, but PyPI does have some structured data, like python version information, last release, version number of the package, potentially a "source" link, authors

things like that

[cold moon]

PyPi API return https://hastebin.com/ajeruyucij.json (example response)

[eternal owl]

Im done with the tags command

I am using fuzzywuzzy to fetch the closest results

[brazen charm]

was that supposed to be seasonalbot and not bot?

[eternal owl]

the name says seasonalbot

but I am working on bot repo

some fine day when I was working on seasonalbot, it decided to rename my bot

and I haven't yet changed the name

[brazen charm]

bot already had fuzzy tags so that's why I'm asking

[eternal owl]

the old code was using regex and I have no idea how to use it so used fuzzy

[brazen charm]

Doesn't seem like the best idea to rewrite something because you don't unterstand it? iirc Shirayuki considered it but decided against as it didn't perform as good as the current implementation

[eternal owl]

by current implementation, you mean using fuzzy?

[brazen charm]

the re

[eternal owl]

okay

I could try to copy the regex code

[brazen charm]

as far as I can see there's no reason to touch the fuzzy matching code for the scope of the issue

[eternal owl]

okay, I will look into it after dinner

[gusty sonnet]

I did test against fuzzy and difflib ( granted fuzzy also uses difflib )

You can see the original snippets used here

https://github.com/python-discord/bot/pull/542

Later on I swapped out to a different algorithm as well

re was used to simply split the string

_targets = iter(REGEX_NON_ALPHABET.split(target.lower()))```was the only place that `re` was used

[woeful thorn]

I'm not sure why we need to touch much beyond the source of the tags

[cold moon]

Sorry, but can anyone please review my SeasonalBot's PR? I just want start new work (I have currently nothing to do...)

[tough imp]

I can take a look tonight

[cold moon]

ok, thanks

[valid quest]

@eternal owl good job

Manjaro is cool

[tawdry vapor]

@cold moon Hey. In general, please don't force push commits to a PR once reviews have started. It can make it more confusing for the reviewers since diffs can't easily be seen and context may be lost for review comments.

[cold moon]

But how should I amend commit? I read this need force push to amend commit.

[woeful thorn]

You don't need to amend the commit

[cold moon]

okay, so just new commit?

[woeful thorn]

Yes

In general you should only be amending commits that haven't already been pushed

[cold moon]

Should I do this like this:

# Return when message don't have attachment and don't moderate DMs
        if not message.attachments or not message.guild:
            return

or making another if check?

[tawdry vapor]

That's fine.

[tawdry vapor]

⚠️

Hurray

[hardy gorge]

niiiice

[brazen charm]

bugged out with the review requests here? https://github.com/python-discord/bot/pull/797

[woeful thorn]

What is bugged?

[brazen charm]

requesting indivifual people like here for example https://github.com/python-discord/bot/pull/790, just wondering

[woeful thorn]

Seems close enough

[patent pivot]

@brazen charm it's intentional

it's load balancing the reviews around the core developer team

[woeful thorn]

I think he was wondering why that one stuck as code reviewers and the older ones did some weird assign/unassign thing

[patent pivot]

oh yeah

huh

fixed

[brazen charm]

Just in case it was a cause for some concern and not a gitub hiccup or something.
What exactly is the sentry for? Monitors and logs errors as they're raised? So i can plan around it more like requested in #797

[tawdry vapor]

Yes. It notifies of warnings and errors

Which happen in production code

It only notifies once for a particular issue, so it won't spam us. Still, we only want to be notified of things that are important.

[patent pivot]

@brazen charm so the error in #797 is BOT-1S on sentry, which for core devs means we can see

normally it has more info like in site errors:

and then unwinds the stack

and lets us see relevant logs

[clever wraith]

so the movie command
there is no check for API limit

@cold moon

[cold moon]

@clever wraith https://developers.themoviedb.org/3/getting-started/request-rate-limiting

[clever wraith]

WTF my check is failing

[cold moon]

@clever wraith Did you pipenv lock?

[clever wraith]

does that generate a pifile.lock ?

[cold moon]

Yep

[clever wraith]

and i thought i have to do that manually

OMG

ok i am not commiting ever again until someone point out the mistake i am doing here

@gusty sonnet

there ?

[gusty sonnet]

Yes?

[clever wraith]

can you tell me why my test are failing ?

I did lint them

locked them

still f them

[cold moon]

WAIT! You added requests?

This is in-built

And also don't use this

[clever wraith]

but it doesn't lint

[cold moon]

It's blocking

[clever wraith]

it says its from outside

[gusty sonnet]

Did you install precommit hook?

[clever wraith]

run precommit ?

[gusty sonnet]

Yes

[clever wraith]

I think so i did

oh wait

Now i know

the one i am cloning is old

and i can't fork until my other PR get approved

I will leave it here
and wait till my other one get approved

[green oriole]

What on earth is that thing? https://discordapp.com/channels/267624335836053506/622895325144940554/682396691537985536

There are a lot of those messages at the bottom of the log too

GitHub.. Stop

[clever wraith]

@crude gyro I am sorry for making one of the worst PR, i think i am not gonna make it. sorry , cuz i don't want to waste your or other reviewer's time pointing out each of my mistake :)

[cold moon]

I can take this over after my current PR get merged

[cold moon]

I need only 1 more approve (seasonalbot)

[cold moon]

Can anyone review my PR? I want start working on new issue.

[molten bough]

They'll get around to it when they have time

Unless the new issue is blocked by your PR, there's no need to wait.

Well, unless you didn't create a branch to work on in your fork

[green oriole]

Nothing is stopping you from creating a new branch from the pydis master though

[cold moon]

Yeah 😅

I started working with Space cog

[green oriole]

@cold moon I think something like SpaceImage would be a better name to avoid confusion

[cold moon]

There is no only imgs. https://github.com/python-discord/seasonalbot/issues/358 @green oriole

[green oriole]

NasaFacts then?

[cold moon]

Maybe just NasaCommands or Nasa?

[green oriole]

The number of cogs of seasonalbot seems to grown quite quickly lately, I think it is better to have some explicit cog names, to avoid some issues we have with weirdly placed commands in the python bot. The cog will just be used to pull random facts from the NASA API right?

[brazen charm]

did the build get stuck here? https://github.com/python-discord/bot/pull/797

[hardy gorge]

Let me check

It did not trigger a build at all

not sure what's up

[brazen charm]

the pr seems broken in some way, didn't request the review proprely either

[hardy gorge]

There were some hiccups yesterday with GitHub in general

[brazen charm]

had trouble with it in the past few days

[hardy gorge]

Anyway, a build is running now

it's done

[clever wraith]

@green oriole screenshots are provided right inside in the issues , and there are 4 issues i.e 4 commands , that uses NASA's API

[green oriole]

Okay, fair enough then

[eternal owl]

I think embeds look better for suggestions?

[green oriole]

!clas

[stable mountain]

Did you mean ...

class
classmethod

[eternal owl]

btw its no matches found... a typo there

[green oriole]

I'm not sure

[eternal owl]

in my img

it says not ....

[green oriole]

Yeah, there is the typo

I mean, I'm not sure if it is really better

[eternal owl]

Okay I think the existing one is simpler, lol

[green oriole]

The -> seems off

Maybe you should try to use bullet lists

[eternal owl]

how?

[green oriole]

Like here https://discordapp.com/channels/267624335836053506/680716760134975491/680755079850360843

I think it is just a unicode point

Maybe using * class will also create the list

Or

!charinfo •

[stable mountain]

Sorry, but you may only use this command within #bot-commands.

[green oriole]

.>

\u2022

[eternal owl]

alignment is a lil off

[green oriole]

Alignment? Wdym?

[eternal owl]

the text and the bullet

they are not on the same level

[green oriole]

Make sense

But I think it is more pleasant that way

[cold moon]

Idea for future: make development guide to PyDis site, where is generally said what things to do and what avoid

[green oriole]

The contribution guidelines?

https://github.com/python-discord/bot/blob/master/CONTRIBUTING.md

[cold moon]

I mean code style

[green oriole]

There are some paragraphs about code style inside the guidelines

What would you see in this guide?

[brazen charm]

pep8, the flake8 setup and what isn't covered in those can be seen in code or get picked up on your first 1-2 PRs

[cold moon]

As one example: loop vs list Comprehensions and other things like this

[brazen charm]

there are not many reasons to use for loops that can be replaced with list comps in any code

[green oriole]

I mean, that's just regular python code good practice

It would be a lot to write about

And nobody would probably read it tbh

[glass pecan]

we did start writing a style guide though

back at the start of december

[green oriole]

_And never finished it? _

[glass pecan]

pretty much

[mellow hare]

People get busy

[glass pecan]

and some details hadn't been decided on

ill clean it up and bring it back up with the rest of the core devs later on

we might get to squeeze some time to review it again

[green oriole]

Nice

[glass pecan]

@brazen charm actually i hate to say that, at least for me personally, there is preferences to use for loops in cases where the readability would be impacted too negatively

a good example would be nested comps

[brazen charm]

I didn't say to always use them, but it is mostly preferrable as can be seen over the projects. If it's branched out more or has a few more levels that can't be taken with something like product a normal loop will be more readable ofc.

[glass pecan]

pretty much, yep. but it's good to be clear especially with newer contribs

just the normal rule of "if it looks like ass, it probably needs to be redone" can apply to most things

lol

[brazen charm]

With discordpy and no use of it outside of contributing here I have needed a basic list comp to access attributes of an e.g. a collection of Members/Channels a couple of times now and a comprehension is a perfect match there

But @cold moon as ELA and I have said, you also need to split up your commits; style things can be picked off by the reviewers if they are somewhat sparse but the git history is important.
You said you've worked 6 hours as the last comment to your open pr, 6 hours of work should not be in one commit. https://github.com/python-discord/seasonalbot/pull/361/commits/c1bc07f5676ba96a2f4fd458fb0c8c19e4eccda7 compare the amount of changes to any of the commits in for example here https://github.com/python-discord/bot/pull/755/commits

[green oriole]

Mark tend to really spam commits haha

[gusty sonnet]

I'm curious, how did it go for you @tawdry vapor

I'm about to test moving channels too in a quick bot

[tawdry vapor]

I guess we can move here, less crowded

First test

1. 48 -> 44; actual 41
2. 45 -> 42; actual 42
3. 47 -> 09; actual 09
4. 47 -> 53; actual 53
5. 48 -> 18; actual 18

Second test

1. 49 -> 42; actual 40 (2)
2. 46 -> 53; actual 49 (4)
3. 45 -> 44; actual 44 (1)
4. 47 -> 12; actual 12 (not present before)
5. 47 -> 09; actual 09 (3)

Numbers at the end in parenthesis are what the channels were numbered in the first set of results

[glass pecan]

it's not spamming commits when they have a clear and distinct reasoning and description

[eternal owl]

Btw, the bot linter doesn't pickup missing doc strings within private methods @glass pecan

[glass pecan]

so?

[green oriole]

Yep!

That’s the mastering of commit haha

[woeful thorn]

They're ignored on purpose, flake8-docstrings doesn't check them

[gusty sonnet]

It's fine to ignore on private methods, but you should put some docstring to it to avoid confusion for others. If it is super clear you can just ignore it

[eternal owl]

Okay

[gusty sonnet]

Most of the time it is used for very specific things

[eternal owl]

Btw shirayuki, u wrote the regex for the tags cmd?

[gusty sonnet]

Yeah, it's a simple regex to split on whitespace / non alphabet

If you are refering to the one used in fuzzy

[eternal owl]

Okay, I need help implementing that

I'm using fuzzywuzzy as I had to write but we had a recent discussion

To use regex as its more accurate

[gusty sonnet]

The discussion was not about that, it's about what you are trying to use it for

What are you trying to do with the tag cog?

[eternal owl]

The same thing, but not using the api anymore

Static files

The outcome will be the same

Output*

[gusty sonnet]

Hmm, then it's a simple switch of source for the tags, right?

Before it was doing api calls to get data, now it's calling something else

Then you should not have to change any of the tag cog

Just the part that's calling the API

[eternal owl]

Well yea, I cleared the whole file and wrote everything from scratch

[woeful thorn]

Why?

[tawdry vapor]

@gusty sonnet I tried moving them more slowly but according to logs the positions are still incorrect for some.

[gusty sonnet]

Hmm, did you change the order of moving as well @tawdry vapor to see if the result is consistent

[eternal owl]

@woeful thorn I'm not sure 😅, but everything is working

[woeful thorn]

That's nice, but it shouldn't require an entire rewrite of the cog

[gusty sonnet]

This is really weird, I'd go into the d.py and put in some logging to save my sanity, if it is doing what it is advertising then it might be discord API

[tawdry vapor]

I may give up on it and open issue for someone else to add the feature in the future

This is becoming a waste of time

Was supposed to be simple

[eternal owl]

@gusty sonnet

Ops miss tag

[gusty sonnet]

Hmm, yeah, just put an issue for it for now imo, and I'll do more tests on my side

[cold moon]

Huh, here is required to split commit to smaller commits, but in one other gitlab repo I need to make 1 commit per pr...

[gusty sonnet]

It's more important to push the rest up since they are functional

[tawdry vapor]

I guess I'll keep my code. I mean it kind of works with some channels 😄

[gusty sonnet]

Functional first! I demand a PR!

[tawdry vapor]

It's no worse than the default ordering

[brazen charm]

The contributing.md says pretty clearly how the commits should look like

Can't really see much purpose behind one commit per pr but not that big of an expert with git

[woeful thorn]

If they want to do it that way it’s up to them. You’ll sometimes see PRs being squashed into one when merging, but generally that comes with a lot of detail in the merge commit message

But squashing, say, all the events of WW2 into one commit with “some bullets were shot” and merging that after “Germany invaded Poland” loses out on a bit of detail

[tough imp]

@cold moon I'd just like to add, a nice clean commit history makes a PR much more easier to review. You can use those commits to show me your thought process and reasoning beind the various choices you've made, and walk me through it. When I was reviewing the Game cog yesterday, it was pretty much just one huge commit and then a couple commits around that I did not understand, and I had to spend a lot of time untangling it and making sense of things myself. I know that you've been asking for reviews for a while now: you're much more likely to get a quick review if you make your PR easy to understand.

[cold moon]

OK, so I should make 1 commit per fix/feature?

[tawdry vapor]

Sort of. That's vague terminology. Sometimes a fix requires a lot of changes, so that may end up being 10 commits.

[tough imp]

You can review this quick write-up, point 4 links an article: https://github.com/python-discord/bot/blob/master/CONTRIBUTING.md

For example, I haven't been able to look at your fixes yet because I didn't have the time to untangle another monolithic commit. If you addressed various parts of the feedback separately, and named / described each commit appropriately, it would have been much easier for me than taking everything in at once

[cold moon]

So like (Games Cog): Moving embed pages to Templates, (Games Cog): Added API key check etc.?

[tough imp]

I think both of those are good examples of things that could have been added in separate commits

[cold moon]

ok

[tawdry vapor]

Need reviews on this https://github.com/python-discord/bot/pull/755

It's blocking Python 3.8 upgrade

[mellow hare]

Looking in on it now

[tawdry vapor]

Thank you I appreciate it

Hope you don't mind the commit I just pushed

[cold moon]

Sorry about asking, but is there any update/had discussion about https://github.com/python-discord/bot/issues/796 ?

[mellow hare]

Uggghhhhhhhh

I swear to god pipenv will be the death of me

[hardy gorge]

Why?

[mellow hare]

The past several times I've tried to use it, it's just caused nothing but headaches for me. I know how to fix it, but it's just irritating

@tawdry vapor Correction, attempting to review it. Nothing is cooperating with me today

[hardy gorge]

Is there anything we could change to make it easier? I've not had any issues myself, but if there's something structural that's causing issues, we should look into that

[mellow hare]

No, this is from me installing different things like Conda and not being careful about what goes where. It's self imposed through trying to help some of our users with their issues.

[clever wraith]

a GUI for pipenv will make it less annoying thats for sure

[mellow hare]

It's not really an interface issue, though

It's just making sure you don't accidentally install it in a version different than the one you're trying to use or what have you

And the fact that it hasn't had a proper release for over a year also makes me uneasy

Yeah, turns out the problem was that Anaconda took over my 3.7 install entirely

So that's a thing

Didn't realize it'd abduct it wholesale like that

[tawdry vapor]

That's alright, thanks for trying if you couldn't get to it

[mellow hare]

Is there any way to tell if pipenv has completely frozen?

[tawdry vapor]

Maybe when that emoji stops spinning

[mellow hare]

Sadly that doesn't seem to show when doing it in PyCharm

[tawdry vapor]

Sometimes docker builds freeze on the pipenv step because of a networking issue with Docker on my end and it just never actually gives up

[mellow hare]

Weird

[tawdry vapor]

So, there isn't always a way to tell.

But when it's done nothing for 10 minutes then it's probably frozen

[mellow hare]

Fair

Yeah, for site I keep getting stuck on 31 out of 80. Are we still on 3.7 for that?

[tawdry vapor]

Yes

Try running it in verbose mode

[mellow hare]

I'm going to try upgrading my venv first

It's currently 3.7.4

Probably not it, but I wanted to get that done anyway

[tough imp]

I still haven't figured out a way to use pycharm with pipenv, lol

[mellow hare]

It used to work flawlessly for me but it's been a pain in the ass for months

[tough imp]

I just handle pipenv via the cli and point pycharm at the virtualenv

[tawdry vapor]

It's supposed to auto detect but I don't reccomend

[mellow hare]

Well even doing the CLI method it's borking on me

[tough imp]

It "works" but it keeps telling me that the requirements in the pipfile arent satisfied, yet when i peak at the interpreter its all there

[mellow hare]

Yep, I get that as well

Real mixed bag

[brazen charm]

How did you add it? I think I just added pipenv when adding the interpeters and worked fine since

[mellow hare]

Which one of us

[hardy gorge]

I've never initialized one of our projects from within PyCharm, but it has always recognized the existing pipenv so far

[mellow hare]

In my cause I'm more than positive it's from all the jumping around on interpreters and everything else I do

[hardy gorge]

Hmm, but wouldn't you just have an isolated environment for the project that should not be influenced that much by the other things you do?

[mellow hare]

The venv itself is yes, but where I've got pipenv instaled is another story

[brazen charm]

@valid quest did you create the branch off of an another one? Got a bunch of commits there

[valid quest]

Yeah i will delete it and recreate it, just saw that

[crude gyro]

@mellow hare I have also had countless problems with pycharms pipenv integration

I just handle pipenv in the terminal and then add the interpreter to the project after I've created it.

that always works fine

[clever wraith]

sorry for nitpicking but,,,,,

https://github.com/python-discord/bot/pull/710

[green oriole]

Done

[crude gyro]

hey @jade tiger, will you be able to take a look at https://github.com/python-discord/bot/pull/519 soon? I know the PR has been open for a long time, but I think it's probably going to be ready to merge as soon as those comments from @tawdry vapor have been addressed. If you poke me when you've addressed it, I'd be happy to review it again as well.

[jade tiger]

Of course. Sorry times gotten away from me - it's been on the to-do list!

[crude gyro]

no need to be sorry

I'm just doing some house cleaning, trying to make sure we get rid of some of these old PRs

[jade tiger]

👍

[crude gyro]

same goes for you @green oriole and this one https://github.com/python-discord/bot/pull/617

both have been open since October.

[green oriole]

Sure!

[eternal owl]

Can any mod/admin send a get request to the website which will fetch all the tags and post the output

[crude gyro]

I believe @glass pecan did this very recently

and added it to the issue

[eternal owl]

Yea I have the gist

but I am wondering if it also sends aliases

[crude gyro]

I think aliases are just handled in the bot?

[eternal owl]

!tags functions-are-objects is same as !tags fao, supports short forms

[crude gyro]

it seems a bit strange that it supports fao

but we do not have a tag in the database called fao, and I imagine this is because of fuzzy matching

see bot.cogs.help.HelpSession._handle_not_found

[gusty sonnet]

It indeed comes from the fuzzy

[glass pecan]

there you go, full raw dump in json

[gusty sonnet]

fao is __functions-are-o__bjects

[glass pecan]

no data on aliases in db

[eternal owl]

yea it looks at starting letter of each word

[glass pecan]

the only thing i removed was embed titles, because they matched general titles

[eternal owl]

!tags if-name-main is same as !tags inm is also same as !tags nm

[gusty sonnet]

it lets you search like if main, will match for if-name-main

Basically if-name-main

[eternal owl]

I guess I will use ur code for this

[crude gyro]

@gusty sonnet I understand that f, a and o are all in that name, but with a score cutoff of 90, I would've thought that wasn't anywhere near enough to match

[gusty sonnet]

fao is a 100 score, all letters are found to be starting of words in the functions-are-objects

[crude gyro]

so is f

[gusty sonnet]

Yep, but we have a gate for query smaller than 3 letters

[crude gyro]

I see

okay I guess that seems reasonable

[eternal owl]

!tags fa works but not !tags fo

[gusty sonnet]

So unless it does a perfect hit ( only 1 tag found )

fo matches too many

!tag -fo

[stable mountain]

Did you mean ...

functions-are-objects
foo

[eternal owl]

oh

[gusty sonnet]

!tag -fa

[stable mountain]

Calling vs. Referencing functions

When assigning a new name to a function, storing it in a container, or passing it as an argument, a common mistake made is to call the function. Instead of getting the actual function, you'll get its return value.

In Python you can treat function names just like any other variable. Assume there was a function called now that returns the current time. If you did x = now(), the current time would be assigned to x, but if you did x = now, the function now itself would be assigned to x. x and now would both equally reference the function.

Examples

# assigning new name

def foo():
    return 'bar'

def spam():
    return 'eggs'

baz = foo
baz() # returns 'bar'

ham = spam
ham() # returns 'eggs'

# storing in container

import math
functions = [math.sqrt, math.factorial, math.log]
functions[0](25) # returns 5.0
# the above equivalent to math.sqrt(25)

# passing as argument

class C:
    builtin_open = staticmethod(open)

# open function is passed
# to the staticmethod class

[gusty sonnet]

fa only matches a single tag, so it is still accepted

fo matches 2, and has length smaller than 3

[eternal owl]

So now in ur code, I have have to populate the self._cache variable and we are good

[gusty sonnet]

In the current code, the part that you will be interested in is the part where it gets all the tags

which is here py async def _get_tag(self, tag_name: str) -> list: """Get a specific tag.""" await self._get_tags() found = [self._cache.get(tag_name.lower(), None)] if not found[0]: return self._get_suggestions(tag_name) return foundand herepy async def _get_tags(self, is_forced: bool = False) -> None: """Get all tags.""" # refresh only when there's a more than 5m gap from last call. time_now: float = time.time() if is_forced or not self._last_fetch or time_now - self._last_fetch > 5 * 60: tags = await self.bot.api_client.get('bot/tags') self._cache = {tag['title'].lower(): tag for tag in tags} self._last_fetch = time_now

[crude gyro]

yeah, you shouldn't need to fuck with any of the fuzz stuff

[eternal owl]

iits get_tags method

not that function

there is another one above

line 35

[cold moon]

@clever wraith I have question about https://github.com/python-discord/seasonalbot/issues/358 : What NASA Open API should I use? I can't find

[gusty sonnet]

Yep, I posted both of them

[crude gyro]

you definitely need to change _get_tags, it's making API requests

[gusty sonnet]

_get_tag() and _get_tags()

[eternal owl]

yea im changing get_tags now

I dont think i have to change _get_tag()

[gusty sonnet]

If your self._cache has the same structure then you can keep _get_tag()

[eternal owl]

yea

[crude gyro]

yeah it looks fine to keep that

[gusty sonnet]

Also make sure to check for other parts that's calling the API

like this py await self.bot.api_client.delete(f'bot/tags/{tag_name}') self._cache.pop(tag_name.lower(), None)when deleting

[eternal owl]

to confirm the struture,

self._cache = {
tag_title : tag_description
}```

okay

[gusty sonnet]

Yep, lower() as well

self._cache = {tag['title'].lower(): tag for tag in tags}```

[eternal owl]

oh yea

[gusty sonnet]

So it's in the form of a Dict[str, Tag]

[eternal owl]

👍

Tag is nothing but the description

[gusty sonnet]

It looks like a dictionary for me

so you can do py await ctx.send(embed=Embed.from_dict(tag['embed']))

[eternal owl]

oh

oh yea

right

[clever wraith]

@cold moon

[eternal owl]

I have need to change that now

[clever wraith]

PR i issued had it usage

although let me tell the one i used

this one for .apod command

this one for .mars

[cold moon]

Apod command is done and others I found. Only NASA facts is what I can't find

[clever wraith]

this one for .nasa

[cold moon]

Ok, thanks

[clever wraith]

this one doesn't need api_key

[cold moon]

Yes

[clever wraith]

html = await fetch(session, 'https://images-api.nasa.gov/search?media_type=image')
            data = json.loads(html)

did i just spammed the message here ?

my internet is not stable at all

[cold moon]

I have to build pagination to there

Due otherwise this return always same result

[clever wraith]

wait wtf

it gave me 99 result when i worked on it

yeah it does give 99

@cold moon

see under items

every data you will need

[eternal owl]

changed the _get_tags() method and everything is working fine now 👍 @gusty sonnet

[cold moon]

I know. I just need make function that grab data from random item.

[cold moon]

When I want to push to different branch in PyCharm, this want include games command commits too. How to remove them?

[glass pecan]

@cold moon on your local git clone, you should make a branch for each feature you're developing, avoid the use of master

[green oriole]

You have to checkout master first, then create a new branch

So the branch is based off the latest commit on master

Although if you haven't created a new branch, you want to checkout upstream/master first, assuming as upstream is the pydis remote

[cold moon]

ok

[eternal owl]

https://github.com/python-discord/bot/pull/803
looks like i got conflicts

[hardy gorge]

That because your fork is outdated and changes were made to the tags cog after you forked:

This branch is 3 commits ahead, 244 commits behind python-discord:master.

[green oriole]

You should work directly on the pydis repo, it would be easier for everyone

[brazen charm]

looks like the build got stuck there too? Or is it blocked when started with conflicts?

[hardy gorge]

Right, I'm not quite sure what's causing the merge conflict; you do seem to have most commits in the history

However, there were some merged commits that affected the file

[green oriole]

Well, we now know why builds are getting stuck

[molten bough]

lol, specifically europe pipelines only

one of the glowstone guys works at MS in the azure department, I'll see what he complains about when he wakes up :>

[green oriole]

Our engineers are currently investigating an event impacting Azure Pipelines causing auto-triggered releases to be delayed for several minutes. The event is being triaged and we will post an update as soon as we know more
Well

The fix for the underlying issue will be rolled out over the next couple of days.
Really >.>

[molten bough]

Weekend

[eternal owl]

is there github dark theme?

[molten bough]

I use Material Dark Github with Stylus

a few things aren't styled though

[eternal owl]

Where can I activate that theme

[molten bough]

using Stylus

for.. whatever browser you use

[eternal owl]

okay

Anyone knows how to resolve conflicts?

[hardy gorge]

If you look at the file with the conflicts in them, you see this:

https://github.com/python-discord/bot/pull/803/conflicts

[eternal owl]

I don't understand what those indicators mean 😅

[hardy gorge]

Now, in this case, it looks a bit magical, for a specific reason: The parts that conflict don't exist in your latest commit, but they do exist in one of the more recent commits you don't have

[eternal owl]

its highlighting the code which I removed

[hardy gorge]

Normally, there's something between the <<<<< tag_overhaul and ======, namely your version, and below the ==== before the part that says <<<< master there's the version in's conflicting with

In this case, your removal conflicts: Mark's recent commit still has it, while yours doesn't, so it does not know which version is "right"

[eternal owl]

So what should I do now

[hardy gorge]

Since you do actually want to remove it, the conflict resolution is to make sure the file looks how you want it be

So, you should just remove that entire chunk

You're basically saying "My version is right; we really don't wan this"

[eternal owl]

done \o/

[clever wraith]

@gusty sonnet there ?

[gusty sonnet]

Yes, I'm here

[clever wraith]

ok so my PR have some changes needed right

so i want you to live time review it right now , cuz i am gonna be busy after a hour for now for several days

[gusty sonnet]

Yes, to avoid the while loop breaking

[clever wraith]

yes

so pass is replaced with removing the person reaction ? just ?

[gusty sonnet]

Imagine something like this

[clever wraith]

so i do some change and you review it right now ,

imagining

[gusty sonnet]

def hello():
    raise ValueError('YOU ARE ALSO NOT PREPARED')

def hi():
    raise TypeError('YOU ARE NOT PREPARED')

try:
    hi()
except TypeError:
    hello()
except ValueError:
    print('Oh no!')

What do you think will be the output of this code?

[clever wraith]

Oh no!

[gusty sonnet]

To your contrary, it's a ValueError

!e ```py
def hello():
raise ValueError('YOU ARE ALSO NOT PREPARED')

def hi():
raise TypeError('YOU ARE NOT PREPARED')

try:
hi()
except TypeError:
hello()
except ValueError:
print('Oh no!')

[stable mountain]

@gusty sonnet :x: Your eval job has completed with return code 1.

001 | Traceback (most recent call last):
002 |   File "<string>", line 8, in <module>
003 |   File "<string>", line 5, in hi
004 | TypeError: YOU ARE NOT PREPARED
005 | 
006 | During handling of the above exception, another exception occurred:
007 | 
008 | Traceback (most recent call last):
009 |   File "<string>", line 10, in <module>
010 |   File "<string>", line 2, in hello
011 | ValueError: YOU ARE ALSO NOT PREPARED

[gusty sonnet]

There are two even

If the code in the except blocks are raised, they are not caught by other excepts

[clever wraith]

TBH i don't have time to learn someting right now , It is a family emergency

can we just pass it on

i can do some edits right now

[woeful thorn]

If you don't have time to do it, then do it later

[clever wraith]

ok

Don't call it stale

[woeful thorn]

We have things we want to do to

[clever wraith]

too* ?

[woeful thorn]

Really

[clever wraith]

yeah i know

[woeful thorn]

If you know, then don't do it

Go take care of whatever else it is you have to do, the bot will probably still be here when you get back

[clever wraith]

i know you guys also have stuff to do

probably*

[woeful thorn]

The point of these contributions is also as a learning exercise, if you aren't able to invest the time to learn something then come back when you can

[clever wraith]

            except discord.Forbidden or discord.HTTPException as exception:
                if exception == discord.Forbidden:
                    await ctx.send(f"{user.mention} Please enable your DM to receive the message.", delete_after=7.5)
                await reaction.remove(user)
            else:
                sent_person.add(user)

will this work ?

[woeful thorn]

Try it?

[clever wraith]

can;t

i can't raise httpexception myself

its not possible

its super RARE

[woeful thorn]

You can manually raise any exception you want

[valid quest]

Use isinstance to compare exceptions

[gusty sonnet]

You can certainly do raise discord.HTTPException inside the try code to purposely go into the except

[clever wraith]

before i can go test it
does that code looks like it match standard of a good code ?

[gusty sonnet]

Except the part where you catch multiple exceptions, it looks good syntax-wise, but probably overly complex

[clever wraith]

do ? ```py
except (Error1, Error2) as e:

[gusty sonnet]

That is much better, yep

[clever wraith]

except (Error1, Error2) as e:
    try:
        ... # handle e
    except Error2 as f:
        ... # handle f

            except (discord.Forbidden, discord.HTTPException) as exception:
                try:
                    await ctx.send(f"{user.mention} Please enable your DM to receive the message.", delete_after=7.5)
                    await reaction.remove(user)
                except discord.HTTPException as exception2:
                    await reaction.remove(user)

tadah ?

[green oriole]

Forbidden isn't a child of HTTPException?

[clever wraith]

maybe ?

yes it is

no

no its not

[green oriole]

!d discord.Forbidden

[stable mountain]

`discord.Forbidden`

exception discord.Forbidden(response, message)```
Exception that’s thrown for when status code 403 occurs.

Subclass of [`HTTPException`](#discord.HTTPException "discord.HTTPException")

[green oriole]

Yes it is haha

[cold moon]

Is this time my commits better https://github.com/python-discord/seasonalbot/pull/364 ?

[green oriole]

You only need to except HTTPException then

[gusty sonnet]

Well, that looks like py try: 1 / 0 except ZeroDivisionError: 1 / 0

[cold moon]

Lmao

[gusty sonnet]

The point is to avoid crashing the loop

[clever wraith]

wonder what will happen

>>> try:
...     1 / 0
... except ZeroDivisionError:
...     1 / 0
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
ZeroDivisionError: division by zero

[green oriole]

Well, an uncaught exception

[gusty sonnet]

You are doing the same thing with py try: await ctx.send(...) await reaction.remove(user) except discord.HTTPException as exception2: await reaction.remove(user)

A log.warning or a log.critical then move on will do fine

remember, reaction.remove() can raise exceptions on its own

[clever wraith]

fuck i just did

            except discord.HTTPException:
                await ctx.send(f"{user.mention} Please enable your DM to receive the message.", delete_after=7.5)
                await reaction.remove(user)
            else:
                sent_person.add(user)```

hahaha

[green oriole]

log.exception would be better here though

[gusty sonnet]

log.error is a nicer way to do log.exception but yes, both are fine

[clever wraith]

i never read anything of log lib

[green oriole]

Are they the same?

[clever wraith]

this all can be fixed if we make exception broad

but lint will not pass

[gusty sonnet]

Avoid broad exceptions

We have a on_command_error for it

If it is exceptions that is very specific, try catching it

[clever wraith]

error_handler you mean

since Forbidden is a class of HTTPException

why not just except HTTP ?

and use inistance ?

to know if that was a forbidden ?

so if that the case

            except discord.HTTPException:
                if isinstance(discord.Forbidden):
                    await ctx.send(f"{user.mention} Please enable your DM to receive the message.", delete_after=7.5)
                await reaction.remove(user)
            else:
                sent_person.add(user)```

nope totally not how it was supposed to be

I will do it later then , as ELA suggested.

[cold moon]

https://github.com/python-discord/seasonalbot/issues/295 Is anyone already working on this or I can start solving this?

[clever wraith]

I was supposed to work on that , but nope i am not going to. . removing the comment

[green oriole]

Edit the comment instead of removing it

You should try to always keep the context

[clever wraith]

A little too late, also no one noticed that comment

[cold moon]

Quick note: https://github.com/python-discord/bot/issues/804 I don't think good first issue and Level 1 - intermediate is good combination.

[brazen charm]

why not?

[cold moon]

I think Good First Issue should lvl 0

[woeful thorn]

Someone working on their first GH issue isn’t necessarily a brand new programmer

[glass pecan]

something can be intermediate in it's implementation but not require heavy knowledge of our projects

[cold moon]

ok

[cold moon]

https://github.com/python-discord/bot/issues/791 Solving this (pagination)

[brazen charm]

Get yourself assigned before jumping on stuff, potentially wasting your or someone else's time if anyone picks it up before you

[brazen charm]

Not sure on the whole reason why tags are being moved, but has it also been considered for things like the doc command documentation links?

[hardy gorge]

The main reason is that we want to use the review system GitHub offers so that people can suggest and review tags

[brazen charm]

Doubt people would really suggest more for the doc inventories, but also applicable there. Just remember it being a bit tedious gathering and adding all of them from here when setting it up locally

[hardy gorge]

There was a project to add default values for dev environments, but I think we should keep things that we want to be able to configure on the fly in the database

I don't really see an added benefit of having a review system for documentation links

People can just suggest them and an admin can add the link if it's something we like

[brazen charm]

the site just contains the links now right? Would also remove the dependency from the site, so it doesn't need to be up for the doc command to work (like when it needed to be refreshed a bit ago when I think sentry was being set up). Doesn't really need to be updated on the fly imo, and wasn't possible to do so until the refresh command was implemented... but just a throught, also kinda agree that resources directly in the bot repo are a bit ugly

[tawdry vapor]

@hardy gorge Using the asyncio.shield for a similar case as you suggested before and I'm getting a warning for a coroutine not being awaited :\

Is it just being dumb? I'm passing the coroutine as a variable rather than directly writing it in the shield.

This doesn't tell me much /opt/pyenv/versions/3.7.5/lib/python3.7/asyncio/events.py:88: RuntimeWarning: coroutine 'HelpChannels.move_idle_channel' was never awaited self._context.run(self._callback, *self._args) RuntimeWarning: Enable tracemalloc to get the object allocation traceback

[hardy gorge]

@tawdry vapor Did you await the shield?

[tawdry vapor]

Yes

I don't always get the warning. I don't know how to reproduce the warning.

Well I'll just have to see if it comes up again

Leaving the log here https://paste.pythondiscord.com/unaxerucan.txt

I restarted the bot while a channel was in-use. It re-scheduled it cause it hadn't been 60 seconds yet. I manually use a command to make it dormant. Then the warning appears.

[hardy gorge]

I don't see you awaiting the coroutine anywhere in that flow, but maybe I'm crazy

Let me show you how I trace it through the execution flow

wait, you do

hm

never mind, I got confused by the different callbacks you defined

[tawdry vapor]

Thanks for looking into it

I guess you looked at the code I pushed?

[hardy gorge]

yes

I need to play around with it a bit, but I'm not using my own machine right now

[tawdry vapor]

At the least, it'd be nice to have a way to log more info about what's going on. Otherwise, even if this does occur again by chance, I still may not be any closer to debugging it. I don't know how to even log it though.

[hardy gorge]

The warning should happen when the object is deallocated, so that somehow happens before it's scheduled with the shield. That's my guess at least.

have you tried logging after the sleep, but before the shield?

just to make sure that shielding there actually happens

[tawdry vapor]

No. I'll add one now

Like here? ```py
log.trace(f"Waiting {data.wait_time} seconds before awaiting callback.")
await asyncio.sleep(data.wait_time)

    # Use asyncio.shield to prevent callback from cancelling itself.
    # The parent task (_scheduled_task) will still get cancelled.
    log.trace("Done waiting; now awaiting the callback.")
    await asyncio.shield(data.callback)

Oh cool I reproduced it

Just followed the steps I did before, which didn't repro it before 🤷‍♂️

I don't see the log I added, so I guess the task is cancelled before it's done waiting?

[brazen charm]

Noticed in #bot-commands that !pep 0 doesn't work because it lives in the base url .../dev/peps. Is that worth making a special case for?

[tawdry vapor]

Sure, I'd welcome it

[hardy gorge]

@tawdry vapor That was my guess, yeah, and this seems to confirm it

So, it's not the shielding in itself that's causing the issue here; it's that we never get to the shielding point

[tawdry vapor]

Most of the time I can cancel the task while it's sleeping and it has no warnings

[hardy gorge]

You did confirm it went to sleep for cases in which is throws the warning?

[tawdry vapor]

Yeah I can see the log message

And it should be the only task currently scheduled

[hardy gorge]

Can you do one hacky thing for me to see if that does anything?

[tawdry vapor]

Sure

[hardy gorge]

        log.trace(f"Waiting {data.wait_time} seconds before awaiting callback.")
        try:
            await asyncio.sleep(data.wait_time)

            # Use asyncio.shield to prevent callback from cancelling itself.
            # The parent task (_scheduled_task) will still get cancelled.
            log.trace("Done waiting; now awaiting the callback.")
        finally:
            await asyncio.shield(data.callback)

Cancellation is done by throwing a CancelledError in the coroutine in the next cycle of the event loop, I'm wondering if the timing of that plays a role. The finally should make sure that the shield happens regardless of the CancelledError

[tawdry vapor]

Well I can try it. But to be clear, it's actually desirable for the shield to not execute if cancelled while sleeping.

Or rather, for the callback to not execute

Well I don't see the warning anymore. But it's kind of confusing what happened since it tries to move the channel effectively twice

[hardy gorge]

right, but, in that case, the coroutine object isn't actually marked as done anywhere, which means it will give you that warning. The warning isn't anything special, it's just that the coroutine object wasn't actually awaited/scheduled. In this case, if the shield does not happen, that's the case (it's never scheduled before this points) and it will throw that warning on deallocation of the object

You can manually call close() on the coroutine object to force it to close and mark it as done

But, if the shielding never happens, you never schedule the coroutine objected assigned to TaskData.callback

[tawdry vapor]

That made sense

What didn't was why it was inconsistent

I tried using the dormant command again, this time without all the restarting bot nonsense and it also showed the warning

[hardy gorge]

I'm not quite sure either

In my mind, the only explanation is that when the warning doesn't happen, the shielding does get called somehow

That or the object is not deallocated, which would be weird

[tawdry vapor]

Gonna see if this works ```py
try:
...
finally:
if inspect.iscoroutine(data.callback):
data.callback.close()

Well I don't see the one for moving channels

But now GuildChannel.set_permissions has the warning

[hardy gorge]

Interesting. Any idea from where?

[tawdry vapor]

Yes it's the other task I schedule

For the 15 minute claim timer thing

Well, that has a bug surrounding it, sort of

So IDK if it's relevant

Since I'm owner of the server, I bypass the revocation of send message permissions

I claimed two channels and it got confused since a task existed

Yeah I think the issue there is that the scheduler refuses to schedule the task since it detects one already exists. So the coro gets disposed like right away - it never even goes into _scheduled_task which has the code above

[tawdry vapor]

These bugs are getting crazy

Hopefully that's it for now

[tawdry vapor]

@clever wraith This channel is a better place to discuss the bot and our other projects

The reminders work by scheduling tasks to run "in the background" and keeping track of which tasks are scheduled (so they can be cancelled or re-scheduled, if needed). The tasks are responsible for waiting until the target time and then sending the reminder.

Pending tasks are also saved to a database for persistence. If the bot crashes or restarts, the tasks can be retrieved from the database and scheduled again, ensuring no reminders are lost.

[clever wraith]

Ah @tawdry vapor thank you

so the database is just a backup

so what if the tasks are more than 48 hours?

[tawdry vapor]

Why would that be a problem?

[clever wraith]

oh nvm

[tawdry vapor]

It's no different

[clever wraith]

I mean't 48 days

so does the scheduling of tasks only happen when a reminder is set or when the bot starts?

I heard from other people that they have a bg task that checks the database periodically to see what reminders should be called

[tawdry vapor]

When the bot starts, it schedules tasks. If a new reminder is created, a new task is scheduled too. If a reminder's duration is edited, the old task is deleted and a new one scheduled.

No, we don't poll the database for tasks. The task just knows how many seconds until the reminder needs to be sent and does an asyncio.sleep

[clever wraith]

right so in this bot's case the database acts like a backup of stored tasks that gets rescheduled each time the bot is started

[tawdry vapor]

Yes

[clever wraith]

Yeah, so how does the call of asyncio.sleep() work?

and how is the task stored?

[tawdry vapor]

It's basically this. Pretty simple: ```py
await asyncio.sleep(duration)

Sleep is done now, so send the reminder

await send_reminder(...)

[clever wraith]

oh sorry, I meant creating the sleep task

[tawdry vapor]

It's done with this method. It's pretty short. https://github.com/python-discord/bot/blob/master/bot/utils/scheduling.py#L34

Basically using asyncio.create_task and then adding it to a dictionary.

[clever wraith]

a dictionary hmm

so the tasks are all stored in a dictionary?

[tawdry vapor]

Yes

That is done to facilitate cancelling the tasks. Otherwise there's really no way to retrieve the task

[clever wraith]

right

so how are the tasks retrieved?

I know as the user you can specify the id

but how is it done when you do !remind list

[tawdry vapor]

From the dictionary, using a key. The key is specified when the task is created.

The list command for reminders uses the database to retrieve reminders

Because the task scheduler uses an arbitrary ID for the tasks rather than the user's ID. This is because users can have multiple reminders, so a user as the key wouldn't be unique.

[clever wraith]

yeah, but how are the arbitrary task id's connected to the user's id?

[tawdry vapor]

Through the database

The bot could also keep track of this information locally, but whoever wrote this chose not to.

[clever wraith]

Like, is the task have a "author" attribute that stores the user id?

[tawdry vapor]

The model is here https://github.com/python-discord/site/blob/master/pydis_site/apps/api/models/bot/reminder.py

[clever wraith]

Ah, is the reminder object itself stored in the database?

[tawdry vapor]

However, the task itself doesn't query the database. The task is given this data when the task is scheduled and it holds onto it.

It's stored locally too

But only by the task

The commands still rely on querying the database

The data can be seen being given to the task here https://github.com/python-discord/bot/blob/master/bot/cogs/reminders.py#L204

https://github.com/python-discord/bot/blob/master/bot/cogs/reminders.py#L226

Line 204 stores the reminder object from the database into the variable reminder

And in line 226 that reminder is passed to the task when creating it

[clever wraith]

Hmm

That's very interesting

[eternal owl]

The reactions are misplaced for some reason

Happens only sometimes tho

[clever wraith]

they were fine for me

maybe network bump

spikes*

[eternal owl]

oh

btw when will there be an announcement for the brand new event

[valid quest]

Mark, pycharm support's colored logs, no?

Anyways I couldn't set it up there, so can colored logs is a good thing to me

[tawdry vapor]

I don't know

[cold moon]

Is anyone already working on this https://github.com/python-discord/bot/issues/562 ?

[crude gyro]

doesn't look like it. I can assign you to it

[hardy gorge]

@cold moon @crude gyro Let's wait with additional unittests until the PR for Python 3.8 is merged as I need to port each additional unittest to Python 3.8. It should not take long, depending on reviews.

Better not to go do double work

[crude gyro]

sounds good

[cold moon]

OK (I already started work, but okay)

[glass pecan]

Keep the work you've done. Nobody is saying to toss out the start of your changes. He's just saying to hold it until the update so you can use the new mock objects rather than it creating another pr for a minor change

[cold moon]

okay

[hardy gorge]

Python 3.8 introduced some changes, nothing dramatic and most of the code will still be the same. It's mainly that I've now been updating each unittest as they were merged and that takes me 10-40 minutes each time depending on the size of the unittest file. Things mostly stay the same, except for async test methods and AsyncMocks.

Since the new AsyncMock objects in Python 3.8 behave slightly different from ours, it sometimes requires a rewrite to port it that wouldn't be needed if you start with the 3.8 AsyncMock te begin with.

So, it's nothing against you, @cold moon, but my time is limited as well

[cold moon]

During waiting, can I work with https://github.com/python-discord/seasonalbot/issues/295 ?

[crude gyro]

I don't see why not.

well

looks like @brazen charm is already working on it?

it's been stale for quite a while though.

maybe I'm missing context.

care to comment, num?

[molten bough]

I hope that coloredlogs PR has an easy way to disable the entire thing

Colorama has never worked right for me on windows

[brazen charm]

Was waiting for a response to the commend and then maybe Python's new help to be pushed but then forgot about it; haven't commented on it because only noticed the unassign so fine for anyone else to grab it

Are you running it through pycharm @molten bough ?

[molten bough]

I mostly would be, yeah, but I've also straight up had issues with it in cmd, haha

[brazen charm]

Probably not going to do anything with color through pycharm

[molten bough]

it seems to work for most people, but I end up with black on black text after the app has finished running

[brazen charm]

if it finds win and tries to do the api calls through colorama which it can't do anything with

If you're running the native terminal by chance, ConEmu§ is a blessing

[molten bough]

I use alacritty these days

I used to use conemu and other forks of it but I found it slow

windows has a big third party terminal problem right now though

[crude gyro]

if it doesn't work for you, I guess you'll write a bug report.

[brazen charm]

Seems to be broken for me on pycharm, probably because of the colorama because when I emulate terminal output to see escape codes and then print it normally, they work

[molten bough]

I've written several

not on colorama, but on windows terminals and MS support, I don't think it's a colorama problem

windows terminals are a bit broken, as I say, haha