#dev-contrib

1 messages · Page 5 of 1

static canyon
#

Tried reinstalling things but that didn't help

modest star
#

I would like to start contributing to anything really, to help me learn more how can I go about it?

wild prism
#

!contrib

stable mountainBOT
#

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

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

Where to start

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

@modest star ^

modest star
#

Thanks, will take a read of all the info etc 🙂

fossil veldt
#

How are you running the tests again, did you only run the single file or all of them?

fossil veldt
#

Where does the files in rules end up being called?

#

nevermind found it, antispam

mellow hare
#

Are there any PRs needing review more than others?

fossil veldt
dusky shoreBOT
fossil veldt
#

I wouldn't really say it's critical though 😅

static canyon
dusky shoreBOT
static canyon
fossil veldt
#

what config file are you using

static canyon
#

Whatever the one for the test server is

fossil veldt
#

hm, screenshot your test config?

static canyon
#

Not on my laptop atm

fossil veldt
#

not sure why you're getting the redis issue though, weird

static canyon
#

I don't think I have a test config?

fossil veldt
#

usually my auto config works (just right click tests folder and run)

static canyon
#

I'm not using Pycharm to run the tests

#

poetry run task test in the terminal

fossil veldt
#

oh? lemme try

#

oh yeah mine is broken as well

#

for poetry run task test

#

I guess the difference is that the poetry test task is with pytest-xdist parallel mode

#

and the pycharm test is not with parallel

vale ibex
#

double check that poetry in terminal is using the poetry env that was setup and updated by pycharm

#

if you run poetry install --remove-untracked in terminal, does it change any packages?

static canyon
fossil veldt
vale ibex
#

When you run the tests, how many runners does it spawn?

fossil veldt
vale ibex
#

interesting, I just dropped mine to 10 and it has the same failures

fossil veldt
#

was your auto higher?

vale ibex
#

Yea mine autos to 32

#

can you try with 4/8/16?

fossil veldt
#

4 -> Fail
8 -> Success
16 -> Fail
32 -> Fail

vale ibex
#

hmmm alright

#

I'll play with xdist scopes and see if I can get it to work

fossil veldt
#

-n 1 works fine

gritty wind
#

I run with 4 locally and it works for me, might depend on your cpu

vale ibex
#

Sooooooo, I have a fix

#

I'm not sure if it's too much of a hack though

#

Scaleios

#

you are judge, jury and executioner

gritty wind
#

There’s no such thing as too much hack

vale ibex
#

well....since all the RedisSession is a singleton, I just put redis_instance = RedisSession(use_fakeredis=True) in tests.__init__.py

#

So a redis session is initialised before any tests run

gritty wind
#

What’s the issue first, I haven’t actually read the messages above

#

Oh I see

vale ibex
#

xdist weirdness, with some number of workers a redis session isn't created

gritty wind
#

Why do you think this is a hack

vale ibex
#

the init file is empty, it just has logs 😅

#

I mean it works

#

fwiw, all the redis tests inherit from this class ```py
class RedisTestCase(unittest.IsolatedAsyncioTestCase):
"""A very good docstring."""

session = None

async def flush(self):
    """Another great docstring."""
    await self.session.client.flushall()

async def asyncSetUp(self):
    self.session = await RedisSession(use_fakeredis=True).connect()
    await self.flush()

async def asyncTearDown(self):
    if self.session:
        await self.session.client.close()
#

I am guessing some of the tests are getting thrown to workers that didn't have this setup

#

which doesn't make sense, since I thought this would be ran before all tests

#

and it's odd that it only works with a certain number of workers

gritty wind
#

Right, now I see why it’s a hack

#

Well, which tests are failing

vale ibex
#

everyone's mileage will vary too, since the number of workers depends on the cores on your CPU

vale ibex
#

it's some combination of the silence tests

#

They are the mains ones that use this super class

gritty wind
#

I wrote a large portion of them, and I’m not sure if I used that class or not

vale ibex
#

you didnm't

#

this class is new

#

Well

#

it's based on a class you wrote

#

I just moved it to base.py so that another test suite could use it

gritty wind
#

The reschedule and unsilence tests don’t seem to use it

vale ibex
#

Maybe connecting this many times isn't a good idea

vale ibex
#

I wonder if the reconnecting is causing the issue

#

I wonder if this is actually caused by each time RedisSession is called there is a small window where cls._instance is set to none

#

hmmm no it shouldn't do, since that metaclass won't do that

gritty wind
#

For some reason, running the test command just floods my terminal with thousands of exceptions ```py
Current thread 0x File "C:\Python\Python310\lib\traceback.py", line 121, in print_exception
KeyboardInterrupt:
print_exception(*sys.exc_info(), limit=limit, file=file, chain=chain)
Windows fatal exception: code 0xc0000374

#

I have no idea what's happening lol

vale ibex
#

lol

#

0xc0000374 is heap corruption

#

nice

gritty wind
#

Seems pyreadline3 was broken in some way, re-installing that fixed it

#

Interesting, I got the test failures you described on my normal setup

#

However, it's definitely unsilence tests

#

That's too much text, but all 7 are unsilence lol

vale ibex
#

lemme try add RedisTestCase onto that

gritty wind
#

It didn't seem to pass with that yet, no

#

Does the asyncsetup run before or after the regular one

vale ibex
#

after

#

Ah, i see why now

#

we do asyncio.run(self.cog.cog_load()) within the setUp function

gritty wind
#

The failures are in the setup yeah

vale ibex
#

which calls through to _rescheulde

#

which has some redis things

#

we could move that to asyncSetUp

#

calling super first

gritty wind
#

That sounds perfect yeah

#

We might need to move some of the other setup too

#

Something like this seems to have done the trick:

    async def asyncSetUp(self):
        """Load cog instance, and populate attributes."""
        await super().asyncSetUp()
        overwrites_cache = mock.create_autospec(self.cog.previous_overwrites, spec_set=True)
        self.cog.previous_overwrites = overwrites_cache

        # Populate instance attributes.
        await self.cog.cog_load()

        overwrites_cache.get.return_value = '{"send_messages": true, "add_reactions": false}'

But it seems we still have one possibly unrelated failure

# 
>       self.cog.notifier.remove_channel.assert_called_once_with(self.text_channel)
E       AttributeError: 'function' object has no attribute 'assert_called_once_with'

tests\bot\exts\moderation\test_silence.py:795: AttributeError

Which should have been created after cog_load

vale ibex
#

yea I'm seeing the same

gritty wind
#

Let me get a debugger in

#

Oh maybe it's the mocks

#

Would they propagate to the asyncsetupup too hmm

#

Nop

vale ibex
#

Yea, I'm guessing the cogload in the async setup is wiping out the autospec

gritty wind
#

Nah

#

We just need to mock the SilenceNotifier

#

The other things since they are instance values remain unchanged

#

@autospec(silence, "SilenceNotifier", pass_mocks=False)

vale ibex
#

Yea, that's the autospec around the setUp func right?

#

Yea

gritty wind
#

With this, my tests are passing again

vale ibex
#

bot#2252

dusky shoreBOT
gritty wind
#

The unsilence test set up only needs to mock the notifier too

vale ibex
#

wait I'm getting not initialised with this setup now

#

on the UnsilenceTests

gritty wind
#

With the one on your PR, or after making this change?

vale ibex
#

is your change to delete the @autospec(silence.Silence, "_reschedule", pass_mocks=False) from the setUp func?

#

if so, both

gritty wind
#

No so, in the normal setUp, we only need _reschedule and Scheduler. In the asyncSetUp, we need only SilenceNotifier

#

Originally, all 3 are mocked in the setUp

#

Silence tests would be the same deal

#

We should probably move this stuff to a base silence test class

vale ibex
#

Yea

#

would autospecs from a parent class be inherited from subclasses?

gritty wind
#

Sort of yes and no, it's not inherited, but the super will be mocked as it was

#

I'm still struggling a bit with a few things that seem to be mock issues in our util mocks

vale ibex
#

hmmmm, slightly stuck on sorting out the asyncSetUp of UnsilenceTests

gritty wind
#

Wait let me show you what I have

vale ibex
gritty wind
#

Not sure if discord voice calls work, I'll try

#

wait a sec

#

Oh oh, discord is telling me I entered an invalid 2fa code

#

Can you hear me?

vale ibex
#

No

#

guessing you can't hear me either

gritty wind
#

no

#

If you want to meet, I can hop on a google meet

#

Or just send diffs lol

vale ibex
#

oh lol

#

we had the same idea

#

DM'd you a link

vale ibex
#

bot#2252 a beautiful pr

dusky shoreBOT
vale ibex
#

made with love from your local <@&409416496733880320> team

gritty wind
#

:)

vale ibex
#

hey @fossil veldt @static canyon could you pull this PR and see if it works locally for you?

#

I've ran this using a few different worker setups, all seems to be working my end

fossil veldt
vale ibex
#

very cool

fossil veldt
#

what was it lol

#

how did specific number of workers make it fail 🥴

static canyon
gritty wind
#

If you have time, drop a review on the PR

timid sentinel
#

I feel like we could do with some standardisation on how we want to do datetime stuff in the bot, or maybe a guide on what to use when. There are often a load of ways that 'work', but i'm never sure which is ideal. For example, which of these do we use when?

  • datetime.utcnow()
  • datetime.now(tz=timezone.now)
  • datetime.now() and then do .replace(tzinfo=None) on what you're comparing it to
  • arrow.utcnow()
  • arrow.utc(tz=timezone.now)

And should we use dateutil.relativedelta or datetime.timedelta?

And what are the differences between arrow.get and dateutil.parser.parse and datetime.datetime.fromxxx?

As well as our various custom utils and converters and other stuff.

vale ibex
#

We could make it standard by saying you always use arrow

#

and if you need a datetime object, just use the datetime prop

thorny obsidian
#

If we could standardize on always datetime-aware that would be fantastic. It helps for testing features where time matters but you're not in UTC

vale ibex
#

i guess one problem with that is that if we say you always use arrow, it means you will need to convert and datetimes from d.py to arrow before using them

vale ibex
#

at least in bot.

#

we use aware datetimes everywhere now, it's just still a bunch of different ways to make them

#

1895 and 1906 were the PRs that were the recent ones

thorny obsidian
#

Okay, because wookie mentioned above datetime.utcnow() which is tz naive

vale ibex
#

Yea, some of the ones wookie mentioned we don't use anymore

#

I think if we're making new datetime objects, we should use arrow

thorny obsidian
#

honestly I'd vote for arrow just because it's tz-aware from the start. It's not the most obvious that datetime.utcnow() is tz naive and I feel like that's something that could slip through a code review

vale ibex
#

making all our helpers only work with arrow will mean that we need to convert to arrow objects from datetime, and then convert back when passing to d.py

vocal prairie
#

urgh, i might have to ask someone else to fix the timezone issues in #2250 because docker has decided that it does not want to play nice no matter how many times i restart my computer

vale ibex
#

that's annoying

#

do you have specific errors if you want to troubleshoot?

vocal prairie
#

this is all i see when i open docker desktop

unknown.png
#

and i restarted maybe 5 minutes ago

vale ibex
#

could you open a terminal and do wsl --list --verbose

vocal prairie
#

in pwsh?

thorny obsidian
#

this happened to me awhile ago and unfortunately the fix was re-installing docker desktop

vale ibex
#

yea any terminal

vocal prairie
# 
  NAME                   STATE           VERSION
* docker-desktop-data    Stopped         2
  nixos                  Stopped         2
  docker-desktop         Stopped         2
  Ubuntu                 Running         2
vocal prairie
vale ibex
#

huh, they are shutdown

#

yea, maybe just reinstall docker desktop

#

if it that list said shutting down or running, the you could do wsl --shutdown

#

(and then open docker desktop)

vale ibex
#

timezones are weird

vocal prairie
#

don't worry about it

#

i detest naive datetimes

vale ibex
#

IG this is partially where wookie's message came from

#

making it standarised will make this easier

vocal prairie
#

i don't like the organization of datetime in the stdlib at all honestly

vale ibex
#

(if it's possible)

vale ibex
#

which is why arrow is nice

cold island
#

So in the same vein never datetime.utcnow() because it's not actually in utc

timid sentinel
#

oh lol I just realised I typed timezone.now 🥴

cold island
#
  • What timezone are you in?
    Yes.
timid sentinel
placid ermine
#

approval on sir-lancebot#1086 👉 👈

dusky shoreBOT
vale ibex
#

I'm fine to add it if someone tests it and it seems to work

#

a quick glance at src, it doesn't look like it exfils my discord token

timid sentinel
#

hm although it does suggest using datetime as the fix in some of the errors, where we might want to use arrow instead

#

inb4 pydis flake8 plugin??

timid sentinel
vale ibex
#

we could fork and change the messages lol

gritty wind
#

Is there a way to get a more informative output on SQL failures in django

#

It crops the SQL to like 15 characters, and you can't see any of the important info

#

Also do foreign key models in django somehow update the python objects, or does that need to be set manually

#

Currently I just return the call to save, so I can easily set it on the linked object

surreal veldt
#

Can anyone check sir-lancebot#1085 please?

dusky shoreBOT
gritty wind
#

Man these site features are really test intensive

#

It's such a pain to write

fossil veldt
#

@timid sentinel @hoary haven different configs of line breaks and periods

#

what do we think pithink

outer oasis
#

That guy sure is spamming a lot
Good thing it's not @fossil veldt

fossil veldt
#

what do you think though, 1 period, 2 period, or 2 with line break

outer oasis
#

What about on the next line down?
Basically your last one without the blank lines

fossil veldt
outer oasis
#

I like that
On a new line so it catches people's attention and doesn't get lost at the end of text, and doesn't have any extra blank space
Could even actually mention the role to draw more attention to it

fossil veldt
#

won't moderator get double pinged then

outer oasis
#

allowed_mentions=None

fossil veldt
#

hm..

outer oasis
#

Oh wait - Can you use that outside of an embed?

fossil veldt
#

but we want to mention the muted guy right?

outer oasis
#

Fair
allowed_mentions=[muted_guy]?

hoary haven
fossil veldt
mellow hare
#

The empty line in between looks better to me

fossil veldt
#

I'll try the non-mention pinged one as well

mellow hare
#

What would be the benefit of doing that, Shen?

outer oasis
#

Mention the role to get pretty colors to draw the readers attention

Tell it not to ping to avoid duplicate pings because you already get a ping in #mod-alerts

fossil veldt
mellow hare
#

Ah, so you're saying - yeah I getcha

outer oasis
fossil veldt
#

so here it is actually

outer oasis
#

Also lets users "see the ping"

mellow hare
#

I'm indifferent either way

outer oasis
#

Though I guess that is the point of adding this sentence in the first place

static canyon
#

But otherwise I like this (definitely prefer the ping part on a new line)

fossil veldt
static canyon
#

Yeah

fossil veldt
#

hmm

static canyon
#

Keeping the ping

fossil veldt
#

or maybe

hoary haven
#

ping for others involved in this issue: @wild prism @vocal prairie thoughts between the above options?

static canyon
timid sentinel
#

I'm fine with the newline between and I like the ping. I don't really have a preference for the exact wording

wild prism
#

does the mention actually ping

hoary haven
#

it does not

vocal prairie
wild prism
#

i guess i'd go with the article

fossil veldt
# fossil veldt

Alright I updated it to the article version as of latest commit ^

fossil veldt
#

@vale ibex should we standardize the use of the flag keyword argument one way or the other? The discord invite is using it while the other 2 are positional.

stable mountainBOT
#

botcore/utils/regex.py line 15

flags=re.IGNORECASE```
fossil veldt
stable mountainBOT
#

botcore/utils/regex.py line 35

re.DOTALL | re.IGNORECASE               # "." also matches newlines, case insensitive```
vale ibex
#

Yea, feel free to do that if you like

fossil veldt
#

since previously the re.search tests all passed the http/https links but it wouldn't work with re.match

vale ibex
#

Cool Cool, sounds good

outer oasis
stable mountainBOT
#

tests/botcore/utils/test_regex.py line 2

from typing import Optional```
outer oasis
#

Do we have a specific style guide banning the new ones, or can they be used going forward?*

vale ibex
#

you can if you want, I don't see the need in replacing existing optionals.

#

Personally I prefer the optional wrapper if it's a simple hint

outer oasis
#

So just developer preference for now? Sounds good.

Actually - Has a deprecation timeline been announced for Optional and Union? They seem to have escaped the deprecation warnings plaguing the rest of the module

vale ibex
#

There's no deprecations for those, no

outer oasis
#

Why?

vale ibex
#

I think in strict 3.10+ codebases the preferred way will likely be | None

vale ibex
brazen charm
vale ibex
#

probably just familiarity if I had to guess

gritty wind
#

I think it’s more explicit about intentions, but that might also be ^

outer oasis
#

Yeah - I'm just wondering why.... Probably because they made the big sweeping deprecations in 3.9, and the | wasn't added until 3.10

fossil veldt
#

wut Thonk

gritty wind
#

You have something outdated

#

Make sure to update your branch

fossil veldt
#

but I literally made the branch like 10 minutes ago

gritty wind
#

You don’t need to use the netlify script locally tho

vale ibex
#

this is in ci

gritty wind
#

Oh right, Chris hasn’t merged my PR yet

fossil veldt
#

that's the ci yeah

gritty wind
#

👀

vale ibex
gritty wind
#

That is also a factual statement

#

Does not make it right

vale ibex
#

was that PR ready? I thought you were making some further changes

gritty wind
#

I think all further changes are now up to site

#

I’m comfy in bed now, but if someone extends the timeout to 15-20 seconds, I’ll review it

fossil veldt
vale ibex
#

major.minor.patch

vale ibex
stable mountainBOT
#

pydis_site/settings.py line 30

TIMEOUT_PERIOD=(int, 5),```
vale ibex
#

this?

gritty wind
#

Nah I’ll link it

stable mountainBOT
#

static-builds/netlify_build.py line 50

response = httpx.get(download_url, follow_redirects=True)```
vale ibex
#

won't line 43 timeout error first?

#

potentially both need a timeout setting I guess, depending on how quickly the artifact is ready

gritty wind
#

Well the request doesn’t wait until the artifact is ready

vale ibex
#

Ah yea, it's workflow pending

#

same thing though

gritty wind
#

All the requests are “instantaneous”, the timeout is because we wait for the GitHub API

#

But yeah, put it on both

#

Or make a client for the entire script

vale ibex
#

yea that's what I'm thinking too

#

would just mean all the timeouts are 3 min

fossil veldt
#

is this good for the changelog pithink

Changelog
=========
- :release:`8.1.0 <16th August 2022>`
- :support:`124` Updated Discord Invite regex to match leading http, https, www
gritty wind
vale ibex
#

doc builder is picky

fossil veldt
#

oh I need 2 new lines right

gritty wind
#

Would it be support, or feature

vale ibex
#

you can test it with poetry run task docs, that'll build the html

vale ibex
#

:P

fossil veldt
#

could be considered breaking also if someone expected match to not match https 😔

vale ibex
#

seems fine to me

vale ibex
fossil veldt
gritty wind
#

Throwing what token?

fossil veldt
vale ibex
gritty wind
#

Run pip install six, or poetry install

gritty wind
#

(We also can’t have secrets)

vale ibex
#

oh

#

interesting

#

I should maybe read the code

#

or not

#

ima go with not

gritty wind
#

Lol it’s a bit of an insane solution

#

The site API acts as a middle man between our build script and GitHub

fossil veldt
#

seems fine I guess

unknown.png
vale ibex
#

@gritty wind

#

site#767 1 review please

gritty wind
#

I’m here

vale ibex
#

thanks

dusky shoreBOT
vale ibex
#

but yea, it looks good :P

fossil veldt
vale ibex
#

nice nice

#

love to see it

#

thanks scaleios

#

your participation is noted

gritty wind
#

I’m sending you a receipt for my services

#

My phone plan renews tomorrow, prompt payment expected

vale ibex
#

Nice

#

congrats!

hoary haven
#

@fossil veldt i promise wookie and i are not just obsessed with full-stops meow3c

fossil veldt
stable mountainBOT
#

bot/exts/moderation/infraction/_scheduler.py line 250

await ctx.send(f"{dm_result}{confirm_msg}{infr_message}.", allowed_mentions=mentions)```
fossil veldt
hoary haven
#

oooooo you got me

fossil veldt
#

I could change it back up I guess, but that was the original form before

hoary haven
#

mm i have no opinion on this

fossil veldt
#

I'll leave it I guess? Since it's joining 3 other messages which may be empty and expecting a shared full stop? pithink

hoary haven
#

yes you can dismiss my comment

surreal veldt
#

Can anyone check sir-lancebot#1085 please?

dusky shoreBOT
timid sentinel
placid ermine
#

🎉

thorny obsidian
#

@timid sentinel how would you handle possible abuse of someone doing a raid opening as many as they can until we're capped at active threads?

#

This limitation is also to try to handle not reaching that cap. It should be on the person who is seeking help to make the thread

#

We could also have a cooldown thread create role which removes the permission to create threads

timid sentinel
#

how would you handle possible abuse of someone doing a raid opening as many as they can until we're capped at active threads?
We could do it similarly to how we do other anti-spam stuff, still having a limit over a range, although a larger one than just one thread

It should be on the person who is seeking help to make the thread
I'm not sure, I think it's easier for someone offering help to think "this channel is currently moving very quickly, and this might require a bit of back-and-forth to fix".

We could also have a cooldown thread create role which removes the permission to create threads
Good point, I think that would be much better than just deleting, although I still think there's a use case for making a couple of threads in a day

vale ibex
#

to support a role removing permission to make threads, we'd need to put that on a channel override, which isn't bad, just would need to be maintained similar to our muted role.

#

we also wouldn't be able to give permission to create threads on a channel by channel basis either. As if we add a channel override to create threads, we can't addd another channel override to remove it

#

as channel override permissions are additive

#

So we'd need to roll it out to every channel

thorny obsidian
#

I think the creating of a new thread is similar to a help channel. You don't open a help channel for someone else

vale ibex
#

sure, but similarly if you open a thread in a topical channel, that takes off and becomes popular, that precludes you from opening a thread elsewhere.

thorny obsidian
vale ibex
#

permissions are additive within the same scope

#

IE if you have one role that removes a perm and one role that adds a perm in the global scope, the one that adds it wins

thorny obsidian
#

I thought the everyone role within a channel could still be overridden by an actual role

vale ibex
#

So you remove the perm in the global scope and then have everyone role allowed in the channel scope?

#

not sure how those two interact.

vale ibex
thorny obsidian
#

It only blocks you for the day though, right?

vale ibex
#

unless the role is given out as a timeout as a result of our anti-spam, rather than blocking you until the thread closes

#

Ah, so the idea is the role would be removed 24 hours after being issued?

#

actually, for implementation detail, removing it at 00:00 utc might be easier

#

and has similar behaviour

thorny obsidian
#

Mhm, it's just to stop people rapid fire opening threads

vale ibex
#

or even have a task that runs each hour and removes the role from everyone might just do it

#

1 thread/h seems fine

timid sentinel
#

I think we could get a similar raid-protection by limiting to say 5 threads a day

thorny obsidian
#

Yeah that's fine, my main concern comes from the abuse/raid angle. So if there's a solution that keeps that in mind I'm fine. I do want to get buy in from Zig and the mod team though

clever wraith
#

Hello folks,

I'm trying to contribute to our community's bot ( First time ) so before I begin, I wanted to ask this:
Can I find environment setup instructions & how to launch the bot somewhere ? Or do I need to figure that out on my own ?

atomic ivy
#

!contribute

stable mountainBOT
#

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

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

Where to start

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

we have a contribution guide

clever wraith
#

Perfect ! Thanks a lot 😄

surreal veldt
#

When trying to run sir lancebot locally, I did poetry run task start and it outputs this 2022-08-17 17:58:32 | bot.utils.decorators | INFO | Starting seasonal task EasterFacts.send_egg_fact_daily (April) 2022-08-17 17:58:32 | bot.utils.decorators | DEBUG | Seasonal task EasterFacts.send_egg_fact_daily sleeps in August 2022-08-17 17:58:32 | bot.utils.decorators | INFO | Starting seasonal task PrideFacts.send_pride_fact_daily (June) 2022-08-17 17:58:32 | bot.utils.decorators | DEBUG | Seasonal task PrideFacts.send_pride_fact_daily sleeps in August
But gets stuck here (these are the last 4 lines it outputted)

fossil veldt
#

Did the bot start?

surreal veldt
#

Oh, I didn't check

#

But it didn't end the command

fossil veldt
#

Well it starts the server so it should keep going until you Ctrl+C to close it

surreal veldt
#

Does lancebot have a reload/load cog command?

cold island
#

.help ext

dusky shoreBOT
#
Command Help

**```
.extensions

**Can also use:** `c`, `cogs`, `ext`, `exts`

*Load, unload, reload, and list loaded extensions.*

**Subcommands:**
**`list `**
*Get a list of all extensions, including their loaded status.*
**`load [extensions...]`**
*Load extensions given their fully qualified or unqualified names.*
**`reload [extensions...]`**
*Reload extensions given their fully qualified or unqualified names.*
**`unload [extensions...]`**
*Unload currently loaded extensions given their fully qualified or unqualified names.*
surreal veldt
#

Thank you

#

In lancebot, I added this line in another class. It accesses the Bookmark class (the commands.Cog)and calls the action_bookmark. But I get this error: ```py
await Bookmark.action_bookmark(self.channel, interaction.user, self.target_message, self.title)
TypeError: action_bookmark() missing 1 required positional argument: 'title'

#

the action_bookmark function is ```py
@staticmethod
async def action_bookmark(
self,
channel: discord.TextChannel,
user: discord.Member,
target_message: discord.Message,
title: str
) -> None:

#

Actually I think I'm missing self

#

I thought it would be done automatically

vale ibex
#

Did you make action_bookmark a static method?

#

static methods don't have self inserted automatically.

#

self is a reference to the current instance of the class, but if you're calling it statically, there isn't an instance being used

#

I think the real question is why are you using that method from the bookmark cog in another cog?

surreal veldt
#

So now I have ```py
class View(discord.ui.View):
...
async def callback(self, interaction: discord.Interaction):
await Bookmark.action_bookmark(self, self.channel, interaction.user, self.target_message, self.title)

So here, I call `action_bookmark`. which is this function
#

not sure what I'm doing wrong with trying to embed the code, but basically action_bookmark calls a function build_bookmark_dm
This seems to raise this error Traceback (most recent call last): File "C:\Users\tenuk\AppData\Local\pypoetry\Cache\virtualenvs\sir-lancebot-gb-kPGWv-py3.9\lib\site-packages\discord\ui\view.py", line 359, in _scheduled_task await item.callback(interaction) File "C:\Users\tenuk\sir-lancebot\bot\exts\utilities\bookmark.py", line 33, in callback await Bookmark.action_bookmark(self, self.channel, interaction.user, self.target_message, self.title) File "C:\Users\tenuk\sir-lancebot\bot\exts\utilities\bookmark.py", line 78, in action_bookmark embed = self.build_bookmark_dm(target_message, title) AttributeError: 'SendBookmark' object has no attribute 'build_bookmark_dm'

surreal veldt
#

I got passed this maybe not in the ideal way

surreal veldt
#

For the solution to the above issue, is this bad? (this is example code) ```py
class Button(discord.ui.Button):
def init(self, func, ...):
self.func = func
...

async def callback(self):
await self.func()

def somefunc():
pass

view = discord.ui.View()
view.add_item(Button(somefunc))

static canyon
#

But otherwise that seems possibly okay

static canyon
#

@timid sentinel thanks for the review on bot#2017, it should all be addressed now 👍

dusky shoreBOT
static canyon
#

The tests on bot#2031 are failing on GitHub but pass locally?

dusky shoreBOT
outer oasis
static canyon
#

I've done git pull etc. to make sure my code matches

static canyon
outer oasis
static canyon
#

Yeah, no clue then

#

I'm using 3.10.6 but that shouldn't matter

outer oasis
static canyon
#

I don't use VSC

#

I use PyCharm

outer oasis
#

CodeSpaces != Dev Containers

#

You can create one directly from the "code" button on github.com, and it opens as a browser tab

static canyon
#

Ah, right

outer oasis
#

Might not be the most comfortable for you, but it would work... at least theoretically

static canyon
#

Err I don't have those "local" "codespaces" tabs

outer oasis
#

Hm... Nevermind then. I have no idea how to get it to show up. It's not technically supposed to be available to PyDis on our current billing plan, but it shows up anyway for me. But I have absolutely no clue what causes that, or how to make it work for other people.

You could try devcontainers, but that's a chunk more involved

static canyon
#

Eh, don't really fancy that

rapid swallow
#

it shows up for me on firefox

static canyon
#

I'm on incognito if that matters

#

Eh, I don't get it anyway

#

So not that

rapid swallow
#

works for me on incognito too

static canyon
#

Just tried on chrome and it didn't show

static canyon
outer oasis
#

I thought it was GA already

static canyon
#

It loads and gives a form to fill out

#

This doesn't feel like the right approach for getting the tests in the same state locally though

outer oasis
#

Codespaces is rolling out progressively on August 11th, 2021 and can be enabled in settings by organization owners for Team and Enterprise Cloud plans. For users in individual plans, we’re extending the existing Codespaces beta. For those in the beta, access will remain and we’ll share updates on what’s coming in the near future.

outer oasis
static canyon
#

Ugh, recreating the venv

#

I've had to do that like 10 times since the 3.10 change

#

But yeah, I can try ig

outer oasis
#

I don't really think the venv itself would be causing something like this, I'd think it'd be more like the pycache or the testing cache, but I'd go ahead and kill it all, just to see

static canyon
#

So should I delete .pytest_cache too?

outer oasis
#

I would

#

That's my thing - I always kill all the caching first, because I hate chasing my tail trying to figure out WTF is wrong, when everything is actually correct, it's just still using the cached version

static canyon
#

Redoing the poetry env atm

#

Have deleted all the caches

#

Including mypy caches just because why not 🤷

#

Rerunning tests now

#

And.... they're still running fine

outer oasis
#

wat

#

hmm

static canyon
#

Welp, I feel like a fucking idiot

unknown.png
#

Apparently I fixed these but forgot to push 🤦

outer oasis
#

nice

static canyon
#

Indeed lmao

#

Don't even remember fixing them 🤷

#

Guess that's what happens when you do coding at like 4am though because you can't sleep

outer oasis
static canyon
#

Nice

#

Looking at the commit, I remember now

#

The issue was just that we weren't setting the created_at attr of the mock incident messages

#

And so msg.created_at was a magicmock thing rather than a datetime object

outer oasis
#

I was wondering how that worked

#

Does any nonexistent attribute just resolve to a MagicMock()?

static canyon
#

I guess so yeah

outer oasis
#

huh

#

I guess that makes sense
Took me a bit to understand what I was looking at though

static canyon
#

Yeah, I seem to remember it took me a bit to realise too

#

Simple fix though at least

vale ibex
#

inb4 the countless messages in here asking when we're updating to d.py 2.0 now that it's released :P

fossil veldt
#

Are there significant changes from the commit we're on?

vale ibex
#

none that would affect us

#

at least for bot & sir-robin

#

sir lance hasn't been bumped for a while, so hasn't got all the async changes yet

surreal veldt
#

For the buttons that sir lancebot uses, are they persistent through bot restarts, or just persistent while the bot is running?

vale ibex
#

They can be made persistent, if you add the view to the bot on startup.

surreal veldt
#

In which event or function do you suggest adding it? I'm not that familiar with discord.py now (i use disnake)

vale ibex
#

The use of persistent views is the same in d.py as it is in disnake.

#

So you can do the same as what you are familiar with in disnake

surreal veldt
vale ibex
#

You could also add it in the bot's setup_hook

#

when to use persistent views is a different question

#

if the view is going to be in a channel that anyone can talk it, we probably don't need a persistent view

surreal veldt
#

do you mind giving an example of that? Never used it before

vale ibex
#

since the view itself is going to disappear in 5 minutes to the history

vale ibex
stable mountainBOT
#

bot/bot.py line 60

self.add_view(JamTeamInfoView(self))```
surreal veldt
#

And the button is for the bookmark message that gets sent to the users DM's

#

so yeah I'd need it

vale ibex
#

Ah right, what would it do?

surreal veldt
#

oh wait, do link buttons need it to be persistent?

#

because there's no callback

vale ibex
#

link buttons don't send an interaction to the bot no

#

So no need for persistence

surreal veldt
#

oh okay. thank you

vale ibex
#

What is the button in the DM going to do?

surreal veldt
#

I changed it a bit - hope it's okay

vale ibex
#

Seems alright. I'd suggest opening issues for changes like that in future though,

#

Just so that you don't waste your time if we don't want the change.

surreal veldt
cold island
#

@surreal veldt what issue are you working on?

#

The one above?

surreal veldt
hoary haven
#

@fossil veldt hey ionite, trying out bot#2234 - are there additional changes to be made for when infractions are edited & DMs resent?

dusky shoreBOT
hoary haven
fossil veldt
hoary haven
#

sorry is the scope of this PR just for the public messages on the server?

fossil veldt
hoary haven
#

i noticed the DM for the original 1 hour mute shows just the "x hours" for duration

#

initially a 1 hour mute, edited to 2h and then 4h

fossil veldt
hoary haven
fossil veldt
#

why did it say duration (duration remaining) in the first place?

#

seems... redundant?

hoary haven
#

don't quote me on this, but i believe it's to account for a potential delay in between the infraction being issued and the database being updated?

#

which is why we always had the X hours 59 minutes remaining everywhere.. right?

fossil veldt
#

ah no I see why

#

the resend command can potentially send the DM hours later

#

so remaining and duration could be different

hoary haven
#

oh yes

#

mk

timid sentinel
#

yeah see my review

fossil veldt
#

ah right I see wookie's review as well

#

the added kwarg of resend makes sense yeah

#

though do we really need the remaining anyways?

#

would total duration and the relative time stamp be enough? Resend or not?

cold island
#

If we have a relative timestamp I don't think we need the remaining string

timid sentinel
#

I get the intention, to make it extra clear that that duration is not from when the message was sent when resending the PR, but I don't mind if we remove it.

fossil veldt
#

pithink I mean I don't really know in what scenario the resend infraction dm really gets called so

#

I'll keep it with the kwarg I guess

cold island
#

@wookie I don't really understand your comment

#

Sigh

#

@timid sentinel

vale ibex
#

mobile user detected

fossil veldt
#

yeah so I'm testing it with the resend kwarg, but since edit infraction doesn't actually send a DM, to do a edit & send dm, the duration would always at least be off

#

since it's 2 separate commands

#

I don't know how often resend is used with duration edits but I assume often together...?

vocal prairie
#

bahahah

#

i thought chris needed two ticks

#

so i approved after the merge ><

vale ibex
#

thanks

#

your approval is noted

fossil veldt
#

current state of doing an edit and a resend dm right after

#

I mean we could add a command argument to the actual resend command whether to send the additional remaining duration...?

timid sentinel
#

I think in that case it makes sense though, it is technically slightly off, as the resend was done slightly after the edit

cold island
#

This seems a bit out of scope, I wouldn't mind considering it for a separate PR

timid sentinel
#

I don't think we really use resend that much tbh, but i'm not that sure

fossil veldt
#

I'll just leave it like this then?

timid sentinel
fossil veldt
#

then it could automatically not use the resend kwarg

#

I don't really have an opinion in this since I don't use this stuff so it's up to you guys 😅

timid sentinel
#

I think it's worth fixing the off-by-1 issue coming up sometimes in the original mute message. I don't really mind if the remaining text after the duration is completely removed, although I think i'd prefer for it to appear but only for infraction resends (which I think was the current intended behaviour).

timid sentinel
#

And I think it makes sense for resends to make it extra clear that the duration is not from when the message was sent, but I don't really mind

fossil veldt
fossil veldt
# fossil veldt

^ current behavior will be as such. No remaining time on initial DM, but only for DMs sent via the resend command

gritty wind
#

I've got 16 minutes to spare, and I want to upgrade site to 3.10

#

Challenge accepted

#

My IDE is already open, so we're practically half way there

#

It's ironic that in my search for a package's source, I went from a commit that migrated from pipenv to poetry, then one which migrated from poetry to pipenv

#

History truly repeats itself

#

Now from setup to poetry

#

Now another one from poetry to setup

#

This is truly a joke

#

@last patio you're as far back as I can trace, why was pyscopg2-binary added, and was it not possible to add it as an extra for psycopg as recommended in the readme

fossil veldt
#

can't escape poetry 😔

vale ibex
gritty wind
#

3 what?

vale ibex
#

version 3

#

we use psycopg2

gritty wind
#

Of what

#

It’s literally in the Readme for the package lol

vale ibex
#

which package?

#

psycopg?

gritty wind
#

!pypi psycopg2-binary

stable mountainBOT
gritty wind
#

Hmm wait was I looking at 3-binary

vale ibex
#

Yea

#

psycopg 3 is recommended to just use psycopg[binary]

#

similar doesn't exist for 2

#

I tried to bump it a while ago but it was non-trivial

gritty wind
#

Oh it’s the top hit for psycopg2-binary

#

Weird

vale ibex
#

I think sqlalch doesn't support psycopg3 yet

#

one of the other deps we use doesnt support 3

#

think it was sqlalch

gritty wind
#

I’ll dig into it later again

vale ibex
#

Yea, psycopg3 is coming with sqlalchemy 2.0

#

which is currently in beta 1, and can't find a reliable release date for it

#

and who knows how long django will take to bring it in

last patio
#

it saves us from compiler dependencies

gritty wind
#

Thanks

#

Why is psycopg3 the top hit when searching psycopg2?

#

thanks

gritty wind
#

Hey could we get site#764 merged, it's making testing a little hard :D

dusky shoreBOT
gritty wind
#

Very simple PR, literally just explicitly sets a bunch of datetimes in tests to match the original test's intentions

#

Thank you Chris

#

And thanks Wookie

clever wraith
#

Hey folks,
I'm curious, where are our site and bots hosted?
And do we have a some sort of CI / CD pipeline for it? ( Kinda a rhetorical question, but I thought I'd ask still)

vocal prairie
#

I believe they're currently hosted on Linode in Kubernetes inside Docker containers.

#

and uhh, there's a .github/workflows folder which has all the ci/cd stuff inside it in each repo

vale ibex
#

yup, all in Linode's k8s engine

#

the workflows contain the CI and deployment triggers

clever wraith
#

Thanks alot!

gritty wind
#

Bumping django up to 4.1 (from 4.0) seems to bring some nasty changes for our test suite

#

The validation done by psycopg is now more strict on null-constrains, but we've been a little laissez-faire when defining the models in our tests

#

Which leads to a few failures here and there

#

Hm, maybe not quite actually

#

It seems to be specifically for auto-generated primary-keys

last patio
#

we can do it scaleios

#

i believe in us

gritty wind
#

Thanks volcyy, I will use your belief to purchase the solution

last patio
#

thank you

fallen patrol
vale ibex
#

We're still figuring out exactly what we want to do

#

the full migration to ansible isn't going ahead now

#

but we are still thinking about using the netcup servers for our infra

#

we'll announce something once plans are finalised

gritty wind
#

I have traced the issue we're running into all the way to the bowels of django, then psycopg, and it seems all through the data is correct. The ID field is never explicitly specified, which is fine since it's supposed to be an auto incrementing field in the database

#

That however does not seem to be the case for some reason

#

Here's the DDL for the ID column on my actual database

id          serial primary key,

And here's the one for the test database

    id          integer                  not null primary key,
#

Not sure if that's relevant at all though, because testing a demo table with that in a separate SQL thing does work as intended

#

(not sure what engine either, it's just a random online runner)

#

Hmm, running the same SQL insert on the test database, and the actual database directly nets the same issues that I'm running into in the tests

#

I.E, it passes on my database, and fails on the test database

#

I wonder if this is something that's changed in how django creates tables

#

Yup, can confirm that did indeed change between 4.0 and 4.1

#

Not sure why it's not documented anywhere, or if this is actually intended or just a bug

molten perch
#

On PostgreSQL, AutoField, BigAutoField, and SmallAutoField are now created as identity columns rather than serial columns with sequences.
That's all I could find.

#

Nothing else, though.

#

Might be totally wrong, but if you modify one of those fields and generate a migration, it might migrate to Identity columns.

gritty wind
#

That is related yes, but it's listed as a feature, not a breaking change, or migration step, so I don't think you're expected to run into issues with it

gritty wind
#

The issue is precisely that it has changed though, they are using a postgres type which does not seem to support auto incrementation for a django field type that's specifically for auto incrementation

molten perch
gritty wind
#

Oh, no that won't actually change either, because the migration runner only tracks model changes (even that very simplistically), it doesn't try to compare

#

I.E, it just checks that the current highest migration number == the migration number it last wrote (which is also stored in the database in a private table)

#

So running migrate won't change it unless it has to perform some operation which recreates the table entirely

molten perch
#

Ah, I see. I have no idea then.

gritty wind
#

I mean, you've pointed out the right thing in the migration guide, but the issue isn't that a table isn't being migrated, but moreso that the change they made does not seem to make sense

molten perch
#

Yeah, yeah, I see that now. 😄
Gonna check out relevant issues, though.

gritty wind
#

Nothing there, at least nothing marked as 4.1

#

Seems to have been recognized as a bug, fixed, pushed to main, but not released yet

#

So perhaps a 4.1.1 fix, which is the upcoming release

molten perch
#

Judging by other issues it shouldn't have caused trouble, I'm no professional though. 😄
I guess, it's not feasible right now, then.

gritty wind
#

Hm, I don't follow the first statement?

molten perch
gritty wind
#

That issue specifically mentions the GENERATED AS portion, which is indeed necessary to make an integer field work as a serial key

#

It's what issue 33919 is saying

#

Here's a minimal repro of the issue

CREATE TABLE api_nomination
(
    active      boolean                  not null,
    id          integer                     not null
        primary key
  GENERATED BY DEFAULT AS IDENTITY
);

INSERT INTO api_nomination
("active")
VALUES
(true)
RETURNING api_nomination.id;

Try running the above with and without line 6

(You can also test it out here online, password aaa https://extendsclass.com/postgresql/a76313f)

molten perch
#

Ah, I see.
null value in column "id" violates not-null constraint

gritty wind
#

Yeah, the current 4.1 release basically makes all the auto field types just regular integer columns

cold island
split compass
#

Hey there, would a PR to fix typos be welcome?

gritty wind
#

It depends highly on what/where the typos are, and how many changes you're planning to make 😄

split compass
#

How about 19 lines throughout 12 files

gritty wind
#

Ehh probably not because it sucks for people who are working on those files in other PRs when they have to merge and figure out random changes

#

We had a PR that was similar in nature a while back (though more extreme), and that was a really bad time

split compass
#

Ah yeah fair point

gritty wind
#

If you can filter it down to things which are exclusively user facing (for example a typo in an article on our site), that's more fine

split compass
#

Alright, thank you 👍

austere hornet
#

Does anyone know what Discord account owns Github account "dannynotsmart"? If you see the comments on sir-lancebot#1034, you'll understand why I'm asking

dusky shoreBOT
timid sentinel
fossil veldt
cold island
#

yeah that part I know

fossil veldt
#

If only the reason is edited then there is no indication as making changes to last_applied would change the duration

cold island
#

but you can't tell if the reason was changed atm

#

so you can't safely drop the "might have been edited" part in resend

#

At least for now

vocal wolf
#

Hey @static canyon, do you think you're going to finish up sir-lancebot#919? I'm going through a bunch of PRs for staleness, and this one is a candidate for being closed for inactivity.

dusky shoreBOT
vocal wolf
#

@spare plaza We did this one together, did you want to complete it? sir-lancebot#1071

dusky shoreBOT
static canyon
vocal wolf
#

I see

static canyon
#

I don't really have the motivation to rewrite the cog

#

So either we merge as-is and perhaps have another PR for tidying the code, or just close ig

vocal wolf
#

hmm

static canyon
#

Reviews are mostly things like this, or rewriting logic

unknown.png
vocal wolf
#

right

timid sentinel
#

If the purpose is just moving stuff around and it's done that without causing new issues then it's probably fine

static canyon
#

Err

#

It might have an issue

#

Not sure if that is the case or not

#

If it is broken, I think just close the PR since I don't overly intend to work further on it now

#

Unless someone else wanted to finish it 🤷

vocal wolf
#

I'll close it for now, but I'll ping hedyhli saying if they want to work on this PR, they can re-open it

static canyon
#

Sounds good 👍

timid sentinel
#

Looks like that PR was meant to be a fairly simple change but got out of hand

vale ibex
static canyon
vale ibex
#

pep8 says to do return None, if you return an object elsewhere

static canyon
vocal wolf
#

What's Shivansh-007's (on github) username in discord?

#

I've forgotten

#

jason

#

where

#

it seems he may have changed usernames

#

wait a second

static canyon
#

They're not longer in the server

#

On their main anyway

#

This is their about me

unknown.png
vocal wolf
#

ah

static canyon
# vocal wolf jason

fwiw I think the account was always Shivansh-007 (currently is) but their nick was jason

vocal wolf
#

right

static canyon
# vocal wolf right

Whilst you're here, can bot#2017 be merged now? It has 2 approvals but one is slightly outdated

dusky shoreBOT
vocal wolf
static canyon
gritty wind
#

What the fuck

vocal wolf
#

I'm not sure what I want to do with my own PR lol

#

?

#

what's up my home dog slice dog

gritty wind
#

django-distill has an actual homepage with actual content and instructions

#

Literally never seen it lmao

vocal wolf
gritty wind
vocal wolf
#

ye

#

it even has conflicts

gritty wind
#

How much are we talking

vocal wolf
#

uhh

gritty wind
#

Are we talking merge 500 commits, or create a new PR

vocal wolf
#

it's a smol PR

#

and the conflict is 3 lines

#

sir-lancebot#1071

dusky shoreBOT
gritty wind
#

Oh it's that one

#

Was there something blocking it

vocal wolf
#

mind blocked

gritty wind
#

Very good

vocal wolf
#

I was pair programming with griff, I don't know what a completed state of the PR would look like

#

pinged him to see if he'd like to take over

#

for those of you who use vscode for python, what are your recommended extensions?

#

I've been using pycharm for a while and I'm seeing if I'd like vscode instead

vocal wolf
#

of course the python extension, but I'm not sure besides that

static canyon
#

I use PyCharm too though so am not of too much use

#

You can always take a look at the most downloaded extensions and see which apply to python development

vocal wolf
#

true

#

I was just wondering if there were any specific ones in which people use for our repos

fossil veldt
vocal wolf
#

lol

sharp crag
#

You add the context menu as a new feature

  • The existing person finishes and merges their existing PR
  • We as a group open a discussion in dev-contrib whether we want to keep both or remove one - for example, @fossil veldt is already expressing interest in keeping both methods functional

@outer oasis

I think you're right - I was just confused because I don't know how to handle 2 PRs for potentially the same feature

outer oasis
#

(I don't do Discord bots, so I have no idea how they work)

sharp crag
#

You're right, a function. And most of the functionality should be the same, so either CTRL C + V or factor the backend logic into it's own function like you mentioned

#

And this is sort of in the same alley, we were discussing using a context menu similar to a bookmark for evals, so you type out your code in chat, right click, eval - what's your input on that?

outer oasis
# sharp crag And this is sort of in the same alley, we were discussing using a context menu s...

Sounds like a good idea to me
It would also let me still run eval if I forgot to put the !e - It's a bit annoying to have to copy and paste into a new message, especially since it doesn't copy the ``` [unless you edit your message]

I remember someone saying that context menus were a bad idea because you were only able to create ephemeral messages, so you couldn't share the output of the eval - is that true?

sharp crag
gritty wind
#

@vale ibex left a few comments on the bookmark PR. Really only the lookup one matters

outer oasis
#

Hi @gritty wind
See above
Do you mind assigning @sharp crag to sir-lancebot#1089?

dusky shoreBOT
sharp crag
outer oasis
#

Rules and regulations and all that, you know

sharp crag
#

Oh yikes. Didn't know that. Will have to stop self assigning myself random stuff then 😳

#

(please don't hurt me core devs)

fossil veldt
gritty wind
#

I can look into the issue tomorrow possibly, not sure yet but gotta go to bed

fiery bay
#

do i need to include all other branches also while forking?

outer oasis
fiery bay
#

ok

outer oasis
#

Alright, I've got to go to bed now
Good luck!

fiery bay
#

ok, good night

sacred fossil
#

Is @dusky shore supposed to trigger for ....hah?

#

It shouldn't, right? I managed to trigger him while having some conversations

static canyon
#

...h

#

Yeah

#

That's supposed to happen

#

When an invalid command is given it'll recommend similar command names

#

...

#

It doesn't for just dots, but will if there's a letter after

sacred fossil
#

But then, this triggers multiple times

#

....hah?

#

There we go

#

Screenshot

unknown.png
static canyon
#

Because the ? is making it display help or something ig

#

But admittedly that's not something we want

#

....hah

sacred fossil
#

Do you think it scales with more .?

static canyon
#

...............hah

#

Nah, it's consistently the same

#

So it doesn't scale

#

No clue what's causing this, but you can create an issue on the repo

sacred fossil
static canyon
#

.hah

#

Even .hah does it

#

There's literally a bug in a recently merged PR or something

#

Maybe sir-lancebot#1064

dusky shoreBOT
sacred fossil
#

.hah

#

Oh god

static canyon
#

Looks like 3 people reviewed it and none of them actually tested

placid ermine
static canyon
#

It's trying to load hacktober stuff based on the error message

static canyon
#

This is why reviews shouldn't be marked as approving when they haven't tested lemon_sweat

sacred fossil
#

How should I name this

static canyon
#

I guess Incorrect handling of invalid command name

#

And then in the description mention that #1064 likely caused this

#

Not 100% sure it did but I'm like 90% sure given that apparently the previous behaviour was just to error saying invalid

#

I'm assuming command.can_run somehow causes a message to be sent to Discord

#

Or something like that

#

There's definitely issues with #1064 anyway, like incorrect type-hints

static canyon
# dusky shore

I wonder if it's possible to just un-merge this and we can add reviews for what needs to be fixed

#

Rather than a new PR

vale ibex
static canyon
sacred fossil
#

sir-lancebot#1090 has been opened @static canyon

dusky shoreBOT
vale ibex
#

Likely better to PR fixes, rather than revert the commits

static canyon
#

Aight

vale ibex
static canyon
# 
                try:
                    if not await similar_command.can_run(ctx):
                        log.debug(log_msg)
                        continue
                except commands.errors.CommandError as cmd_error:
                    log.debug(log_msg)
                    await self.on_command_error(ctx, cmd_error)
                    continue```I suspect this is the problematic code. We should just ignore the error
vale ibex
#

which is likely handled by our error handler

static canyon
#

Yeah

static canyon
surreal veldt
#

Can anyone check sir-lancebot#932 please?

dusky shoreBOT
full fractal
fading galleon
vale ibex
#

our site requires python 3.9

#

oh wait, static-builds

#

So I'm not entirely sure the setup here

#

I think you may need to run python3.9 to run the static task

#

since it's part of our site project that requires 3.9

#

but the build script itself can only run on python 3.8 due to netlify not supporting higher

#

is that right @gritty wind ?

fading galleon
vale ibex
fading galleon
#

thanks, works now

vale ibex
#

nice nice

gritty wind
#

If site added 3.8 support, or netlify added 3.9, we can build directly on netlify

static canyon
#

I'd say maybe review but just leave as a comment rather than an approval. That way you're still giving feedback, but it won't mean merging without being tested

cold island
#

@timid sentinel wait, but why do we need the "remaining" part if we already have a relative timestamp?

unknown.png
timid sentinel
timid sentinel
cold island
#

hmmmmmmmmm

timid sentinel
#

I mean, I don't really mind 😄

cold island
#

I think it was just the old behavior for all notifications, no?

#

Not specifically resend

timid sentinel
cold island
#

It's also undergone a few major merges from main

#

So maybe something got jumbled up

cold island
#

Yeah I think that's probably what happened

timid sentinel
cold island
#

Are you sure it's not from the current PR? This just checks if there's an expiration and if it's active

unknown.png
timid sentinel
#

The if duration != remaining bit

cold island
#

hmm yeah

#

I think we can remove it anyway lol

timid sentinel
#

Fair

cold island
#

The current solution looks good compared to the current state, so it's not a blocker either way

#

I'll just test it

fossil veldt
#

For all DMs it always sent both duration and remaining time

#

Or rather it adds remaining time if the total duration is not exactly equal to the time the DM is sent. Which, considering the server latency it will never actually be equal? pithink

fossil veldt
cold island
#

If you're doing that I think we should put "expires" after "duration"

fossil veldt
#

Like in the embed order?

cold island
#

yeah

fossil veldt
#

Right yeah

#

That makes sense

fossil veldt
#

since that is inferable now from infraction data

fossil veldt
# fossil veldt

yeah I think I'll do this, it seems significantly more useful than a hard coded duration delta

cold island
fossil veldt
#

instead of

unknown.png
cold island
#

Can you make it just say "edited"?

fossil veldt
#

okay yeah

clever wraith
#

Do we ping people for a code review or just wait until it gets reviewed ? (It's my first time :B)

fossil veldt
#

do we still want the (maybe edited) message then?

# 
reason += "\n\n**This is a re-sent message for a previously applied infraction which may have been edited.**"
cold island
#

mhm, we don't have info about reason editing yet

fossil veldt
#

🙃

#

would be nice to support that but we'd need a new field at least

cold island
fossil veldt
#

even the current edit detection is only possible since we've hijacked the old inserted_at field

clever wraith
dusky shoreBOT
cold island
#

Ah, yeah it might take a few days until it gets attention

timid sentinel
#

I'll take a look when I get a chance

clever wraith
#

Thanks folks!

fossil veldt
cold island
#

👍

fossil veldt
#

alright committed in dcd946b

cursive relic
#

It's finally here (the option to delete bookmarks) thank you 🎉
(If it was there before I just didn't see it)

static canyon
cold island
#

@fossil veldt remind me, did you add inserted_at to the post request for new infractions?

fossil veldt
#

It now sends the same time for inseted_at and last_applied on the initial post

cold island
#

ah ok, couldn't find it in the code for some reason, I see it now

surreal veldt
#

If I was making changes to x file locally and now the code to that file on github has changed, what should I do?

#

For more context, I was making changes to the bookmark.py file prior to the PR of removing bookmarks was merged

static canyon
#

If the change on github has been merged to main, then just merge main and resolve any conflicts

surreal veldt
#

What happens to the changes I made if I merge main

static canyon
#

They stay unless there's a conflict

#

If there's a conflict the editor will ask you to write what it should now be (your version, main's version, or a mixture -- you edit the code to resolve the conflict and then tell git it's fixed)

cold island
#

Since it's your first time dealing with a potential conflict, maybe copy your local bookmark.py somewhere so that you don't lose anything

surreal veldt
#

And how do I merge main? From github itself right?

static canyon
#

git merge main probably

#

Make sure to update main first

#

Or you can do from github itself, but that's generally not recommended since github can't handle large conflicts

surreal veldt
#

Whats the command to update main?

static canyon
#

git pull main maybe

surreal veldt
#

Okay thanks

static canyon
#

I just use PyCharm buttons to do the commands for me tbh

#

git merge main might just work on its own actually

surreal veldt
#

Also, is it okay if i use black fornatter on the file when PRing it?

static canyon
#

Err don't do that

#

We use flake8

#

If you do poetry run task lint it'll tell you about any linting things that need to be changed

surreal veldt
#

Whoops. I had it do it automatically on saving a file

#

Also, when I did the command it said already up to date

static canyon
surreal veldt
#

Git merge main

#

although if it helps, I'm on another branch

static canyon
#

Yeah, then you need to update your main first

#

Do you have any uncommited changes on your current branch?

#

If not, do git checkout main and then git pull

cursive relic