it shouldn't
#dev-contrib
gritty wind
There's nothing else relevant here, and running the command again was fine
!docs cleardoccache discord.py
I think I don't have discord.py as a symbol
vale ibex
hmmmmm
I can't repo that error
I wonder if the warning log is being treats as an error for some reason
oh
it's just a warning with stack info
gritty wind
I have a feeling it has to do with the ratelimit because that's what it's right beside, but I don't know how to reproduce a ratelimit
cool
Another unrelated one, I'm pretty sure this was already there. Just need to remember to fix it
Traceback (most recent call last):
File "C:\Users\hassa\AppData\Local\pypoetry\Cache\virtualenvs\bot-QP3Q7ay1-py3.10\lib\site-packages\discord\ext\tasks\__init__.py", line 239, in _loop
await self.coro(*args, **kwargs)
File "C:\GitHub\PyDis\bot\bot\exts\fun\off_topic_names.py", line 56, in update_names
channel_0_name, channel_1_name, channel_2_name = await self.bot.api_client.get(
ValueError: not enough values to unpack (expected 3, got 0)
I don't have any OTs in the DB
gritty wind
There's a good chance I wrote that PR
It's been so long so I opened the bot lol
How did the expire in docs work before you converted it to an int
was it an int in 3.9
doesn't look like it
vale ibex
it's a new thing in aioredis 2.X
vale ibex
if it's a float, it assumes you're giving it ms
gritty wind
Yikes
vale ibex
so *1000
gritty wind
I'm collecting any undocumented changes and I'm trying to get them to document them
This is helpful
vale ibex
yea, the aioredis 1.3 -> redis-py migration is rough
stable mountainBOT
aioredis/client.py lines 1848 to 1855
def expire(self, name: KeyT, time: ExpiryT) -> Awaitable:
"""
Set an expire flag on key ``name`` for ``time`` seconds. ``time``
can be represented by an integer or a Python timedelta object.
"""
if isinstance(time, datetime.timedelta):
time = int(time.total_seconds())
return self.execute_command("EXPIRE", name, time)```We could in theory just pass the TD
Where does it multiply by a 1000
Seems to have been the same behavior since the file was created
vale ibex
hmmmm
I found a reason for it
since passing a float would give out of range errors
we could pass it as a dt actually, yea
vale ibex
`aioredis/client.py` lines 1848 to 1855
```py
def expire(self, name: KeyT, time:...
this is aioredis fwiw, not redis-py
not that there's much difference, just extra options
gritty wind
The redis-py implm looks like this too, just with the added options which don't apply to us anyway
vale ibex
yea, can't find the page in my history too
gritty wind
Don't worry, it's fine
Is using variables from outside a nested function not allowed
vale ibex
bugbear is warning against https://docs.python-guide.org/writing/gotchas/#late-binding-closures
gritty wind
I feel like that'd produce a lot more false negatives than positives, but I guess that's what bug bear is about
gritty wind
This PR is a lot smaller than I expected
gritty wind
How did you even find those lol
They moved the docs to a completely different, very ugly page
vale ibex
went through browser history around when I made the commit
but yea, this isn't applicable anymore
gritty wind
So this is removed behavior?
vale ibex
we use the seconds elsewhere in that file, so I'm tempted to leave it as-is
or make it a float again
gritty wind
It's fine as is
vale ibex
So this is removed behavior?
Yea, I think it is
vale ibex
This PR is a lot smaller than I expected
most of it is tests
like kaizen fixes of tests
because I was annoyed at warnings
gritty wind
It is very clean now, I'll give you that
vale ibex
yea it's nice
gritty wind
https://aioredis.readthedocs.io/en/v1.3.0/mixins.html#aioredis.commands.GenericC...
Reading it, it says that floats are converted to milliseconds, and passed to a function which does use milliseconds, so I don't think it's changed behavior. It's weird however you were getting range errors
What we have now is correct, so I'll just drop it
With that, I go to bed
Thanks Chris
stable mountainBOT
It has arrived!
fiery bay
why python discord bots are using discord.py (no offense just asking reason 🙂 )
subtle kraken
why python discord bots are using discord.py (no offense just asking reason 🙂 )
Back when they were written it was only prominent framework for python bots
fiery bay
ok
stable mountainBOT
It has arrived!
surreal veldt
Is making a command to display open github issues not something worth it?
outer oasis
Is making a command to display open github issues not something worth it?
To list all open issues?
thorny obsidian
That would be a lot of issues depending on the repo
surreal veldt
Oh I meant unassigned ones my bad
If there are alot, maybe just the most recent x ones
placid ermine
outer oasis
Why do you want to do that in Discord?
surreal veldt
fair, initially i thought it may help when you want to contribute especially to ones doing it for the first time but nevermind
hoary haven
i think the idea has some merit. a lot of us have the repos already in our browser history or bookmarked, but a command that fetches issues based on approved but waiting assignment, or up for grabs might be a quick easy way to show a new contributor what to look for
there may be other use cases as well internally like for internal org or admin repos. or fetching stale issues from meta
actually probably wouldn't be able to use it for private repos, ig
surreal veldt
what about this issue https://github.com/python-discord/sir-lancebot/issues/1023
is the user who said they want to work on it still going to be waited on, or can I request to do it?
atomic ivy
ig you could request to be assigned seeing as there ware no comments after that
surreal veldt
okay, done
gritty wind
For future reference, issues have to be approved before anyone is assigned. You can ask us to take a look if it’s not been approved yet. Anyway I’ve gone ahead and approved and assigned tou
surreal veldt
thanks
When I forked sir lancebot, I get this pop up saying it's not protected. Just to make sure, i should click it right?
fossil veldt
When I forked sir lancebot, I get this pop up saying it's not protected. Just to...
well that's just your own branch since you forked it
surreal veldt
Also what happened to the embeds?
fossil veldt
the branch protection rules aren't copied over
you probably don't have to protect it unless you want to PR to your own fork before PRing to the pydis sirlancebot
outer oasis
you probably don't have to protect it unless you want to PR to your own fork bef...
(but don't PR to main on forks)
surreal veldt
for https://github.com/python-discord/sir-lancebot/issues/1023, which category jokes should I use from the API? Allow the user to chose from 'neutral', 'chuck', 'all'?
static canyon
for https://github.com/python-discord/sir-lancebot/issues/1023, which category ...
Is this the pyjokes API you're referring to?
surreal veldt
yeah
static canyon
I'd say either let the user choose or "all"
At a quick glance none of them seem inappropriate
Maybe default to "all", but the user can specify if they wish to
Feel free to ping me for a review when you're done too btw
@surreal veldt
static canyon
Eh I mean up to you really
static canyon
Should it be in an embed? I assume not, right?
I don't think it really needs to be one in this case
Especially if you're only displaying the joke
surreal veldt
Lastly, is it worth making a class for the list of categories and handling if the input is not an option, then typehinting the argument in the command to the classs?
static canyon
Eh could be nice to be more explicit on possible values
Although the typehint would be misleading
Since you'd be saying what you expect and not what it actually is
I think just doing JOKE_CATEGORIES = {"chuck", "neutral", "all"} at the top of the file and then checking if the category is in that set would be better (so a typehint of str) @surreal veldt
surreal veldt
out of curiosity why a set?
static canyon
Or since it's a cog self.joke_categories
static canyon
out of curiosity why a set?
Because it indicates you'll never have duplicates & it also has the benefit of faster in checks (the latter not really mattering but a nice bonus)
surreal veldt
oh, if you're doing if x in y, the operation is fastest if y is a set?
static canyon
Yeah
By quite a bit iirc
!timeit ```py
x = {"neutral", "chuck", "all"}
print("n" in x)```
stable mountainBOT
@static canyon :white_check_mark: Your 3.11 timeit job has completed with return code 0.
500000 loops, best of 5: 649 nsec per loop
!timeit ```py
x = ["neutral", "chuck", "all"]
print("n" in x)
@static canyon :white_check_mark: Your 3.11 timeit job has completed with return code 0.
500000 loops, best of 5: 746 nsec per loop
So about 15% faster in sets than a list here
For the command though the time difference doesn't really matter because it's still "very quick", and is thus more of a bonus
fossil veldt
oh, if you're doing `if x in y`, the operation is fastest if `y` is a set?
yup so for a list it's O(n) average
if using a set, the in check is O(1) average
apparently there's a worst case of O(n) but I'm not sure how that happens
static canyon
apparently there's a worst case of O(n) but I'm not sure how that happens
I guess when the item isn't in the set?
surreal veldt
So I tried to test the code locally first but my bot isn't responding to me but it looks right so I'll just PR it
should it be in the fun.py cog?
static canyon
.src fun
dusky shoreBOT
static canyon
.src Fun
dusky shoreBOT
static canyon
I think its own file might be better
surreal veldt
just two functions is okay?
static canyon
Depends on how many lines the code is
surreal veldt
import discord
from discord.ext import commands
import pyjokes
class Jokes(commands.Cog):
def __init__(self, bot : commands.Bot) -> None:
self.bot = bot
self.JOKE_CATEGORIES = {"neutral", "chuck", "all"}
async def send_joke(self, ctx : commands.Context, *, category : str) -> str:
joke = pyjokes.get_joke(category=category)
await ctx.send(joke)
@commands.command()
async def joke(self, ctx : commands.Context, category: str = "all"):
if category not in self.JOKE_CATEGORIES:
raise commands.BadArgument(f"`{category}` is not a valid joke category")
await self.send_joke(category=category)
def setup(bot):
bot.add_cog(Jokes(bot))
Yeah that should be fine to go in the Fun cog
I'd change send_joke to _send_joke though to indicate it's a util
In fact I don't think we even need a separate function for it seeing as it's only 2 lines anyway
I'd keep it all in the command personally
It's not like we'll be running that code anywhere else
Also make sure you do the correct import order
surreal veldt
Yeah fair, I'm just used to using functions for some reason
static canyon
Don't think it's a big thing either way but generally functions are used to either prevent duplicate code (which won't happen here) or improve the flow of code (and I'd argue it doesn't help for just two lines)
static canyon
```py
import discord
from discord.ext import commands
import pyjokes
class Jok...
import stdlib
import stdlib2
from stdlib3 import foo
import piplib
import pilib2
from piplib2 import bar
from local.package.module import spam```this is the correct order of imports (and blankline usage) for our projects btwI.e. your from discord should go after import pyjokes
Otherwise it'll give a linting error
We also include docstrings for all of our commands so that they get a description in .help, so can you add one please -- something along the lines of py """Retrieves a joke of the specified `category` from the pyjokes api."""
@surreal veldt ^
surreal veldt
Got it
surreal veldt
@static canyon I made the PR https://github.com/python-discord/sir-lancebot/pull/1081
How do I link sir lancebot issues like how I do bot#1234?
dusky shoreBOT
vale ibex
How do I link sir lancebot issues like how I do `bot#1234`?
sir-lancebot#1081
dusky shoreBOT
vale ibex
The format is organisation/repo#number
for python-discord repos, you can skip thee organisation
Also, the CI is failing cause it can't find the module pyjokes
you will need to add it to the pyproject.toml
poetry add "pyjokes=0.6.0" should do it.
surreal veldt
Can I manually add it to the file?
surreal veldt
Sure, after doing that you'll need to run `poetry lock`
Because when i run poetry commands it says Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases.
vale ibex
Do you have python installed?
surreal veldt
Yeah
vale ibex
Cool, could you disable the shortcut as it says in the error?
surreal veldt
Although I believe there's something with wrong with my vscode interpretor
vale ibex
Windows ships with a command python that when ran opens the windows store to Python install
which conflicts with running the normal python command on the command line
surreal veldt
Although I believe there's something with wrong with my vscode interpretor
because if I do pip install something it says that it installed but when I click the run button in vscode, it says module not found error. However if I run the script with the command from terminal it works
vale ibex
Yea, that will likely be because pip is pointed at a different environment that python is
or at least the version of python your vscode is using
surreal veldt
do you know how I can fix it?
vale ibex
Cool, could you disable the shortcut as it says in the error?
have you done this?
surreal veldt
yeah but then this PS C:\Users\tenuk\sir-lancebot> poetry lock 'python3' is not recognized as an internal or external command, operable program or batch file.
vale ibex
a new error = progress :D
now that you have removed that shortcut I recommend reinstalling poetry
surreal veldt
with pip?
vale ibex
No, by following this https://python-poetry.org/docs/master/
surreal veldt
So i used curl -sSL https://install.python-poetry.org | python3 - --uninstall
vale ibex
are you using wsl?
surreal veldt
and it says 'python3' is not recognized as an internal or external command, operable program or batch file
oh oops
using windows
vale ibex
Yea, then just use python not python3
surreal veldt
also if I am using python 3.10, and just writing python in terminal says 3.9, that's bad right?
vale ibex
that means your python 3.9 install has priority over your python 3.10 install
you can change that by changing the order they appear in your PATH config
surreal veldt
well it still says C:\Users\tenuk>python Python 3.9.7 (tags/v3.9.7:1016ef3, Aug 30 2021, 20:19:38) [MSC v.1929 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information.
surreal veldt
the PATH thing?
vale ibex
Yea
vale ibex
after you change your path you need to restart all command lines
so that your path is reloaded
surreal veldt
i closed my command prompt and opened it again
vale ibex
Do you have vsc running?
surreal veldt
nope
also, if it helps, the Desktop/Coding/python at the bottom in the screenshot is this. Does it mean it's py 3.9?
gritty wind
Move the 3.10 scripts file to the top as well
vale ibex
Yea, that's your python 3.9 install
gritty wind
Second to last here
surreal veldt
Yea, that's your python 3.9 install
how can i uninstall it?
gritty wind
The bots run on 3.9 anyway, why are we trying to get 3.10 to work 😅
You can uninstall it from command panel
Control panel*
vale ibex
Yea, having both installed is fine
surreal veldt
oh my bad, this was more of a personal problem because my vscode interpretor seems off
vale ibex
once you install poetry and setup a venv for the bot, you can set vscode to use that anyway
Did you reinstall poetry yet?
surreal veldt
yeah
vale ibex
Cool, open sir-lancebot in your command line
surreal veldt
done
vale ibex
poetry env use C:\Users\tenuk\Desktop\Coding\python\python.exe
this will create a poetry env using your 3.9 path
surreal veldt
okay - Creating virtualenv sir-lancebot-gb-kPGWv-py3.9 in C:\Users\tenuk\AppData\Local\pypoetry\Cache\virtualenvs Using virtualenv: C:\Users\tenuk\AppData\Local\pypoetry\Cache\virtualenvs\sir-lancebot-gb-kPGWv-py3.9
surreal veldt
Package operations: 73 installs, 0 updates, 0 removals```cool
that means your venv is ready
if you do ctrl+shift+p in vsc, it will open the quick menu
in there, search for select inte
surreal veldt
oh by the way I didn't add pyjokes to the toml file
vale ibex
and click select python interpreter
then go to C:\Users\tenuk\AppData\Local\pypoetry\Cache\virtualenvs\sir-lancebot-gb-kPGWv-py3.9 and select python.exe
surreal veldt
sorry what do I do?
surreal veldt
oh okay i think i did it
vale ibex
it will open a file selector window
cool, in vsc do Terminal > open in terminal
do that activate the new venv?
surreal veldt
i don't see open in terminal option
surreal veldt
done
vale ibex
Did it run the activate script?
surreal veldt
vale ibex
cool, cool, that's worked
surreal veldt
now do I add pyjokes to the file?
vale ibex
`poetry add "pyjokes=0.6.0"` should do it.
you can do these things now
I also suggest running poetry run task precommit
to setup the pre-commit ilinting hooks
surreal veldt
oh and also i forgot to create a new branch before making updates, is that bad?
vale ibex
which updates?
surreal veldt
modifying the sir lancebot code
vale ibex
ahh right
what you can do is make a new branch now, with that commit in it
then switch back to main and reset it to upstream/main
surreal veldt
if I proceed now, does it do anything bad though?
surreal veldt
yeah
vale ibex
it'll mean that after your PR is merged, you'll need to reset your fork to upstream/main
surreal veldt
how do i make a new branch with the commit I made in it? because I already PR'd it
fallen patrol
(its worth making the change to a new branch if you haven't opened the pr yet)
(but if you've opened the pr just don't change it)
surreal veldt
i only have to do the changes to the pyproject so I guess there's no point
i think i did it
sir-lancebot#1081
dusky shoreBOT
surreal veldt
also, what happened to the embed? why is it bigger than it should be?
vale ibex
i think i did it
did you manually add it to the file?
anyway, you will need to run poetry lock to update the lock file and then push that
the lock file is what is used by poetry install the pyprojcet.toml describes what versions we are happy to use when creating the lock file
gritty wind
Ideally you use add, since lock and update like to bump every single dependency
But it’s not a big deal
vale ibex
`poetry add "pyjokes=0.6.0"` should do it.
Yea :D
surreal veldt
I uses the command
vale ibex
Cool, if it's locked now, you can push the lock file and it'll pass CI
gritty wind
Maybe you just didn’t add the lock file to your commit?
There’s a file called poetry.lock in the repository which should have some changes
surreal veldt
I need poetry lock for that?
atomic ivy
git add poetry.lock
If you used the poetry add command, it’ll already have made the changes
atomic ivy
basically stage the changed poetry.lock file
surreal veldt
I do git add .
atomic ivy
and was there a poetry.lock file in the staged changes?
surreal veldt
Okay now it's done
static canyon
```py
import stdlib
import stdlib2
from stdlib3 import foo
import piplib
import...
@surreal veldt the pyjokes import still isn't in the right place. You've put it in the "stdlib" imports group, instead of the "pip" imports group
Also, can you add a description and add Closes #{issue_number} under the "Related Issues" header of the PR
And when you're committing things, ideally each commit should have a different name
vale ibex
now that you have poetry setup, you can run poetry run task lint and it will autofix everything it can
static canyon
And when you're committing things, ideally each commit should have a different n...
E.g.
"Add jokes command"
"Add pyjokes to pyproject.toml"
"Add pyjokes to poetry.lock"
That way it's easy to know what changes you made without having to even look
It's a bit late to change now so don't worry about it but something to consider for futute
surreal veldt
now that you have poetry setup, you can run `poetry run task lint` and it will a...
so I get this error https://paste.pythondiscord.com/ezezanidaj
surreal veldt
E.g.
"Add jokes command"
"Add pyjokes to pyproject.toml"
"Add pyjokes to poetry....
ahh got it my bad
surreal veldt
<@751814647346102303> the pyjokes import still isn't in the right place. You've ...
so like this right? ```py
import json
import logging
import random
from collections.abc import Iterable
from pathlib import Path
from typing import Callable, Optional, Union
import pyjokes
from discord import Embed, Message
from discord.ext import commands
from discord.ext.commands import BadArgument, Cog, Context, MessageConverter, clean_content
from bot.bot import Bot
from bot.constants import Client, Colours, Emojis
from bot.utils import helpers
Try doing $env:SETUPTOOLS_USE_DISTUTILS = 'stdlib'
then running again
The problem and proposed workarounds are described here https://github.com/pre-commit/pre-commit/issues/2178#issuecomment-1002163763
surreal veldt
i believe it's done now
vale ibex
i *believe* it's done now
Yup! looks good
Just had 1 comment as Discord.py has a built-in way to do the thing you were doing :D
Also, can you update your PR description to use the things it's asking for
surreal veldt
Does lancebot have a number guessing game by the way?
vale ibex
like a higher or lower type game?
surreal veldt
Yeah
vale ibex
Not as far as I know
but lance does have many things that I don't know about lol
Feel free to open an issue if you wanted to suggest it
surreal veldt
Also, can you update your PR description to use the things it's asking for
Also mind elaborating on this?
vale ibex
In the PR template we include comments on things we want in the description
the bits between <!-- -->
we expect the description to be filled out as it mentions
surreal veldt
Where do I find that?
vale ibex
Are you not logged in?
If you log in, it should show an edit button in the top right
it also shows this when you first open the PR
surreal veldt
since it's easier here, hope it's okay, is this what I should do? ```py
async def joke(self, ctx: commands.Context, category: Literal["neutral", "chuck", "all"] = "all") -> None:
"""Retrieves a joke of the specified category from the pyjokes api."""
joke = pyjokes.get_joke(category=category)
await ctx.send(joke)
trying it now, although, i assume it'll raise a BadArgument which the bot will use in the error handler and send an embed giving the raw error right?
vale ibex
Yup :)
gritty wind
It'll raise BadLiteralArgument, but it'll still be handled
vale ibex
Yea, I think we handle all BadArgument subclasses the same
just by checking if they're an instance of BadArgument
vale ibex
Agree agree
surreal veldt
okay done
vale ibex
okay done
What's this tatus file in the PR?
looks almost like a git status command but piped to a file
surreal veldt
huhhh i did do git status may have accidentally being put to the file
wait, i'm not sure what did I do
is that a new file I made, can I delete it?
vale ibex
yup
do git rm tatus
this will delete the file from your system and also stage the file deletion, ready for you to commit the change
surreal veldt
the joke command doesn't work?
gritty wind
.joke
It’s still deploying
You’ll see a message in #dev-log from the bot when it’s live
atomic ivy
say, when I restart the bot container locally, why does it send the available help channels message again? unlike how it works here
vale ibex
we store those messages in redis
atomic ivy
just curious
gritty wind
The current message is stored in redis
atomic ivy
ah
gritty wind
Chris too quick
atomic ivy
so if the redis storage is made persistent, it won't do that, right?
gritty wind
Yeah
atomic ivy
cool
gritty wind
We tried fetching it, but it gets complicated because of the api and the other message in #❓｜how-to-get-help
Something along those lines
It’s been a while
surreal veldt
so the bot here never has never restarted?
gritty wind
so the bot here never has never restarted?
It does restart, but it does use redis
So it can find the old one
static canyon
so like this right? ```py
import json
import logging
import random
from collecti...
Yep, although it seems pointless have a from discord.ext import commands when there's also a from discord.ext.commands import ...
gritty wind
Hey @vale ibex the deployment was skipped
static canyon
Yep, although it seems pointless have a `from discord.ext import commands` when ...
I'd be inclined to remove the prior
gritty wind
The PR is merged 😅
static canyon
Ah nvm then
atomic ivy
actions shenanigans?
round kayak
hey
static canyon
Bad reviewers smh /s
vale ibex
sick
gritty wind
Bad reviewers smh /s
100% Chris
static canyon
Lmao
vale ibex
will pusha commit
vale ibex
Can’t see why on mobile
ddoesn't say why on desktop either
just that it was skipped
it even skipped a fresh commit on main that I just pushed
vale ibex
arthur deploy restart sir-lancebot
radiant merlinBOT
:white_check_mark: Restarted deployment sir-lancebot in namespace default.
vale ibex
This should re-pull latest too
gritty wind
It didn’t build so it won’t redeploy the image
vale ibex
Was it renamed
was what renamed?
gritty wind
The lint action name
gritty wind
I mean from the API
It does some funky stuff to give it a url valid name
As opposed to the actual name
vale ibex
Ahh right
I mean it's just "Lint" so I'm not sure how badly you can mess that up
I can check after my call though
Alternative things to check is if these strings are still the ones used ```
if: github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.event == 'push'
success was valid up until a couple days ago so I hope it’s not that lol
vale ibex
I imagine I'd see much more noise across the interwebs if they decided to change that lol
surreal veldt
so this is a github issue?
gritty wind
Yup, we’re trying to figure it out
gritty wind
Well it's working on my fork
I think it's just github being github
Yeah seems like the API was returning malformed data for a bit
Chris are you re-running righ tnow
Who's doing that 👁️
gritty wind
Amazing that it'll happily say invalid value, but not what the value is
vale ibex
yea odd, usually that gets skipped if the build was skipped
dusky shoreBOT
Chuck Norris burst the dot com bubble.
gritty wind
@surreal veldt it shouldn't have taken this long, but it is live now. Thanks!
gritty wind
This is 90% of what it means to be devops lmao
vale ibex
^^ DevOps trying to beat GitHub into submission
very good timing on this Kat
just as cluster networking goes down lol
thorny obsidian
lmfaoooo
surreal veldt
Shouldn't commands like the fun commands only be limited to #bot-commands?
vale ibex
Shouldn't commands like the fun commands only be limited to <#26765994508681216...
Are they not?
surreal veldt
No I dont think so
vale ibex
Could you show me an example of where it's not being blocked?
surreal veldt
Well I didn't see it anywhere but I (personally) thought its something that would be done
vale ibex
Yea, it is done, we have a global check of sir lance
which is why I asked for an example, since it would be a bug if that's not happening
fossil veldt
Is bot#2217 something that could be approved to work on?
dusky shoreBOT
fossil veldt
Or is it still unsure what style we want to format it to?
static canyon
I personally quite like the idea of replacing with a relative timestamp
thorny obsidian
Is bot#2217 something that could be approved to work on?
I think @cold island had some opinions of replacing with discord timestamps because of how it renders on mobile?
cold island
Discord timestamps are a great idea which was terribly implemented
fossil veldt
I think <@512354988157173763> had some opinions of replacing with discord timest...
I think I agree with that as well yeah
Probably just working out the phrasing?
Mainly the relative timestamp's "in" sounds weird as it should really be "for duration"
austere hornet
Also what happened to the embeds?
Yeah I'm seeing the same thing, anyone know why this is happening?
gritty wind
What’s the issue?
outer oasis
Also what happened to the embeds?
Here's an embed from almost a month ago.
As far as I can tell..... They're exactly the same
austere hornet
What’s the issue?
The embed is like stretched out and does not at all look like what Shenanigans sent above (compare the ss sent by Raven and the one sent by Shenanigans)
gritty wind
This is not something from our side, maybe discord is changing the style
austere hornet
Oh alright
cold island
I think I agree with that as well yeah
Discord mobile will literally not show you relative timestamps, it'll appear as dates instead
fossil veldt
<t:1658961000:R>
cold island
Did they fix it? 🤔
fossil veldt
Seems to work now?
vale ibex
Did they fix it? 🤔
on iOS and some android beta iirc
I don't have it on my old phone that's running android stable
fossil veldt
atomic ivy
DST? 
vale ibex
Timezones shouldn't affect absolute deltas lol
cold island
I think the platforms do different rounding
vale ibex
Yea, it's fun
fossil veldt
it's technically 2 hours 55 mins as of right now
some platform implementation is rounding that to 2 I guess
atomic ivy
Timezones shouldn't affect absolute deltas lol
lol yeah. I just saw an hour diff and thought DST. mb
vale ibex
lol yeah. I just saw an hour diff and thought DST. mb
I do not accept your apology, please have a 5000 word apology on my desk by Friday
thanks.
cold island
In any case, I think a pretty non-controversial way to deal with the issue raised is making the response say the intended duration
fossil veldt
In any case, I think a pretty non-controversial way to deal with the issue raise...
yeah I think that might work better for phrasing the response as well
vale ibex
Yea, I think saying the intendeed duration in static test is good
since we already have a timestamp for the expiry date anyway
hoary haven
i found this interesting, while playing around with editing infractions and using infraction resend
notice the new duration of 10 minutes and 19 seconds, because it took me 19 seconds to edit the infraction and resend it
would we just have it say 10 minutes going forward? (i think that's fine)
vale ibex
Yea, the duration should be the absolute time between creation and expiry
fossil veldt
Currently I think the issue is the data type / record does not store the creation time / duration
It just stores the expiry time as far as I can see
so the human time rendering module has to calculate the time difference from expiry until now, which results in that literal duration
hoary haven
Yea, the duration should be the absolute time between creation and expiry
in the screenshot above the absolute time between creation and expiry is 10m19s. it's an edited infraction
vale ibex
in the screenshot above the absolute time between creation and expiry is 10m19s....
if you didn't edit the duration, then the inserted_at time should still be the same
if you edit the duration, then I'd expect the inserted_at to be updated to the new time
cold island
inserted_at is the time in which the record was added
vale ibex
Change it to when the infraction is created/edited
So the bot tells site when it was created
that way we can calc the difference
vale ibex
Otherwise it will always be off, since expiry_date is calculated by bot, and inserted_date is set by postgres some time after
cold island
If we wanted that I'd rather have a duration field
vale ibex
we could add a created_at time
cold island
That's what inserted_at is though
vale ibex
inserted_at is when postgres inserts it currently
fossil veldt
Duration field might be good. If only recording start and end time it will be off for DST switchover days for example.
vale ibex
Duration field might be good. If only recording start and end time it will be of...
all times are in UTC
fossil veldt
all times are in UTC
Ah okay I see
vale ibex
So inserted_at will always have the slight difference to the actual creation time, so can't be used to calcualte the absolute delta
atomic ivy
can't just the expiry_date be updated?
cold island
inserted_at is when postgres inserts it currently
It just seems weird to me to add a new field and to start editing "inserted_at" just so that we could calculate the delta. Let's just store the delta
vale ibex
can't just the expiry_date be updated?
we can't calculate a duration without a start and an end
atomic ivy
So inserted_at will always have the slight difference to the actual creation tim...
does that slight difference matter?
vale ibex
It just seems weird to me to add a new field and to start editing "inserted_at" ...
I'd mostly suggested to change inserted_at to be more like created_at
vale ibex
does that slight difference matter?
yes, it's what is this whole issue is trying to solve
atomic ivy
uhh. mb again. what issue?
vale ibex
bot#2217
dusky shoreBOT
fossil veldt
uhh. mb again. what issue?
The infractions display being off slightly
Since the time calculation is not based on the actual sending time of the command
cold island
The infractions display being off slightly
Just for context, it's not off, it says how much time is left
It's just that 99.99% of the time it's not useful info
vale ibex
since I'm not entirely sure what the purpose of inserted_at is currently
cold island
Since it sends it immediately
cold island
since I'm not entirely sure what the purpose of inserted_at is currently
It tells us when the infraction was applied
atomic ivy
📨 👌 applied mute to
@Examplefor 1 day until June 10, 2022 8:00 AM.
does this suggestion posted in that issue have any demerits?
vale ibex
It tells us when the infraction was applied
if we stored the duration in the db, and didn't change how inserted_at works at all, then editing an infraction's duration after the fact would look odd,
Since the expiry is calculated on date infraction was edited+expiry
But when we resend an infraction the inserted_at date is what is used for the created
So the insertion time would still be the original insertion time, the duration would be the new duration and the expiry would be the time the infraction was edited+the duration
So the difference between expiry and inserted would be more than the absolute expiry
cold island
if we stored the duration in the db, and didn't change how inserted_at works at ...
The resend doesn't say when it was applied
It just says duration and expiry
vale ibex
Ah right, so it would just be the moderation list commands that would be off then
vale ibex
Should we store when it was last edited?
cold island
It was applied at X, but it was edited so now the expiry is Y
This is the current state isn't it?
vale ibex
Yea
cold island
Should we store when it was last edited?
There was a whole project to redo the infractions table schema to include this kind of context, but it didn't take off
I think the issues still exist
somewhere deep under the pile of newer issues
vale ibex
hah
cold island
let me check
vale ibex
Would your suggestion be to store the current duration in a number of seconds?
cold island
yeeeah probably
There's a duration field type though I think in pg
I think we used it for the new filter schema
But yeah it basically gives you the duration in seconds
vale ibex
ah, does it use postgres's interval type?
cold island
I can... check
models.DurationField in django
I have no idea what it means for pg
stable mountainBOT
pydis_site/apps/api/models/bot/filters.py line 40
infraction_duration = models.DurationField(```A field for storing periods of time - modeled in Python by timedelta. When used on PostgreSQL, the data type used is an interval and on Oracle the data type is INTERVAL DAY(9) TO SECOND(6). Otherwise a bigint of microseconds is used.
Yea interval on postgres
nice
I think adding a duration field seems fine, maybe with a last_edited field too, that way we can solve the issue above
can be a separate thing if needed though
cold island
I think that calls for a bigger overhaul. The old plan was to have a command that gives more info beyond the summary in !i s
vale ibex
I am going to guess the schema rewrite was to add a whole audit log for all edits?
cold island
yep
vale ibex
`pydis_site/apps/api/models/bot/filters.py` line 40
```py
infraction_duration = ...
oh, I just noticed that this is already for the infraciton duration
cold island
That's the filters
vale ibex
do we just, not do the issue above until the new filter schema?
cold island
Not infractions
cold island
yep
fossil veldt
we can't calculate a duration without a start and an end
I guess we technically don't need to add an entry to the post call at all? The actual sending of the infractions message still has the original infraction dictionary
https://github.com/python-discord/bot/blob/4af4f0a083550386f781b7626f0f7a3d45934166/bot/exts/moderation/infraction/_scheduler.py#L119
stable mountainBOT
bot/exts/moderation/infraction/_scheduler.py line 119
async def apply_infraction(```if the infractions dictionary included the absolute time the command was called, it can be passed into time.format_with_duration for a (fairly) accurate duration calculation
vale ibex
Yea, that was my suggestion for a created_at field, to store when the infraction command was called, but that won't work for edited infractions without more fields/work
So currently the suggestion is to add a duration field to the infraction model, to store the current duration
cold island
I might have an idea for a new way to show infractions, will try to create a mock-up over the weekend
vale ibex
I think there's a bunch of suggesetions in #mod-meta from a while ago when this was last discussed 😅
cold island
yep ik
cold island
There's a thread I think
@vale ibex thinking about it some more... if we store the "last applied" date instead of duration that will also easily tell us whether it was edited
vale ibex
Should we store when it was last edited?
Yea, sorta what I was suggesting here
cold island
yeah i know haha
vale ibex
I think we could
cold island
that or "last edited" either is fine
vale ibex
and just initialise it with the command ran time initially
cold island
yeah
vale ibex
@fossil veldt are you familiar with django?
since this issue will include some site changes
nothing complicated, just a new field+migration
fossil veldt
<@233059161401720832> are you familiar with django?
not really 😳
vale ibex
Are you interested in picking it up for this?
If not I can make the site changes for you
fossil veldt
If not I can make the site changes for you
yeah um, that might be good 
vale ibex
Sure I can PR it in a sec
fossil veldt
or could you walk me through the stuff I need to do
not really sure how the migration works for django
vale ibex
python manage.py makemigrations and it just auto-creates the migrations by comparing the current models in code vs a full migrated db
site#751
dusky shoreBOT
vale ibex
All I did was make the change in the infraction model, then I started up the app so it created a fully migrated db
then I ran python manage.py makemigrations and it auto-generated the migration file
fossil veldt
then I ran `python manage.py makemigrations` and it auto-generated the migration...
oh okay, so it made that 0084_infraction_last_applied.py file?
static canyon
oh okay, so it made that `0084_infraction_last_applied.py` file?
Yep
I remember it being pretty easy when I had to do it for the dm_sent field
I.e. site component of bot#1864
dusky shoreBOT
fossil veldt
what procedure actually runs the migration file for the server?
vale ibex
Yea, django automatically runs the migrations on startup
fossil veldt
ah okay that makes sense
vale ibex
so is this expected to be updated on edited infractions as well?
Yea
Ah, there is a problem here, and I'm not sure how to resolve it
The last_applied date we want to use is the same "now" date used to calculate the expiry
but that is created in the Duration converter
and not accessible by the command handler
vale ibex
We could change the "now" var in the convertor to now = datetime.now(timezone.utc).replace(microsecond=0) and then do the same in the very first line of the command
and hope that they don't cross a second barrier
vale ibex
could it just store a new utc now as the last_applied date?
any difference would cause the humanise function to return something "weird"
IE if the duration was 2hours, but we used a time that resulted in the infraction being saved with a duration of 1hr59m569s, then it would be shown like that to the user
Actually, something that would liekly be easier is change that converter to use the DurationDelta convertor instead
which returns the actual time delta
then we an do that logic in the command instead, and have access to everything we need
vale ibex
Nah, that's some custom convertors in bot.converters
cold island
Actually, something that would liekly be easier is change that converter to use ...
that's what I'm thinking too. One caveat is that we can also specify the expiry in ISO
vale ibex
that's what I'm thinking too. One caveat is that we can also specify the expiry ...
Yea, we'd need to Union with ISODateTime like the current Expiry type does
and then isinstance check to do the now+duration if needed
or calc the delta otherwise
cold island
mhm
fossil veldt
so currently the Duration type is the datetime of expiry?
vale ibex
The Expiry convertor that is used by the infraction management commands is a union of Duration and ISODateTime both of which return a datetime object after converting
vale ibex
What we're suggesting is making a new Union ```py
InfractionExpiry = t.Union[DurationDelta, ISODateTime]
yea
vale ibex
What we're suggesting is making a new Union ```py
InfractionExpiry = t.Union[Dur...
using this then we can do an isinstance check for relativedelta or datetime to see what the format is, and go from there
relativedelta -> we have the delta, and need to calc the expiry date
datetime -> we have the expiry date and need to calc the delta
gritty wind
Could you store both and calculate nothing
vale ibex
We are going to store both in the db
we need to calculate in the command though, since we only expect mods to give us one of the two
vale ibex
well, if it isn't your's, and it can't be mine, the mods are a good choice
fossil veldt
yea
I assume the conversion is done pretty early right, like in the command functions?
vale ibex
I assume the conversion is done pretty early right, like in the command function...
Yea
it's done before the command is called by d.py
vale ibex
Could you store both and calculate nothing
while you're here site#751 thanks
dusky shoreBOT
vale ibex
very complicated django
tawdry vapor
How's this different from inserted_at
vale ibex
it will be set by the bot to when it was last edited
the default is just there for backwards compat
IE if an infraction is changed, last_applied will be updated to when that happened
tawdry vapor
Oh okay
vale ibex
so the duration+expiry makes sense
gritty wind
Can we make it default to the created column
Instead of now
So it’d be accurate retroactively
cold island
well, if it isn't your's, and it can't be mine, the mods are a good choice
*me's
vale ibex
uhhhh, I'm pretty sure those are calcualted at the same time
well, not literally the same time ofc
we could set it to be server_default
and then it would be afaik
gritty wind
I mean set it for what the current values in the DB is
gritty wind
So an infrac from last week would get that column as last week
tawdry vapor
Should this get transferred back to the site repository? https://github.com/python-discord/api-old/issues/12
fossil veldt
is bot not compatible with running on 3.10?
vale ibex
not until bot#2229 is merged
dusky shoreBOT
vale ibex
you don't need to have multiple poetry installs
outer oasis
Multiple?
vale ibex
if you have multiple python versions installed, poetry can work across them
you can do poetry env use path/to/python.exe
when creating the poetry env
and it will use that version to create the env
cold island
Should this get transferred back to the site repository? https://github.com/pyth...
ooooh so that's where this was
fossil veldt
you can do `poetry env use path/to/python.exe`
can it be a pyenv install? Or does it not matter
vale ibex
cold island
I'd been looking for that several months ago
vale ibex
I mean set it for what the current values in the DB is
Added a commit to site#751
dusky shoreBOT
vale ibex
I think doing an update like this is fine
I think only bulk_updates need to have batch considerations right?
vale ibex
I've tested it, and it works
I'm just not sure about scalibility
the docs don't mention anything about worrying about batching
whereas they do for bulk_update
gritty wind
We can pull the prod database and run the migration, but we don’t have that much data
We’re on like 100K infracs?
vale ibex
I mean if it works on prod I don't care about more than that
gritty wind
Yeah lol
vale ibex
I'll test it out
gritty wind
Can Arthur trigger BB
vale ibex
yea
I'll just pull and push to my local postgres, rather than testing directly in prod
It's arthur cj trigger blackbox fwiw
cj for cronjob
gritty wind
I see
gritty wind
I'll just pull and push to my local postgres, rather than testing directly in pr...
Yeah
Log in as me 👌
vale ibex
always do
vale ibex
meh
vale ibex
lol
vale ibex
We can pull the prod database and run the migration, but we don’t have that much...
lol it worked
took like .5 s
vale ibex
~65k infractions fwiw
fossil veldt
so currently the existing records will be updated with a inserted_at field? What would the value be?
vale ibex
Existing infractions already have an inserted_at field
fossil veldt
oh I mean the uh last_applied
vale ibex
we're adding a last_applied field, and setting that to be the same as inserted_at on upgrade
fossil veldt
oh so the old last_applied records are just a duplicate of inserted_at?
fossil veldt
so displaying old records should just use the last_applied record, which will just be slightly off but still work
vale ibex
It's just so that the bot doesn't need to do special handling of NULL values
gritty wind
It’s accurate too, since we don’t have any way to change it currently
So it’s not off at all
Assuming last applied is when it was last sent
vale ibex
it's not 100% accurate
since duration/expiry is calculated at command time
inserted_at is calculated as insertion time
it's why the DM messages are a few seconds off atm
well, a second off
gritty wind
I meant it’s accurate to what kind of value to expect
vale ibex
so it shows 1hr59m instead of 2h lol
gritty wind
But yeah it’d technically be a second off you troll
vale ibex
so displaying old records should just use the `last_applied` record, which will ...
yea, so it will only really work for new infractions, which is fine since old infractions are only seen by mods
Once the site PR is merged, you can run docker compose pull to pull the latest site image locally
then you can use the new field
fossil veldt
yea, so it will only really work for new infractions, which is fine since old in...
but new infractions must now supply the last_applied record always right, or it wil be null?
vale ibex
yea, new infractions, and infraction edits should set that
gritty wind
Who’s your lol
gritty wind
Right
vale ibex
until then we need the default otherwise any new infractions will error :D
gritty wind
Eh we don’t need to remove it
fossil veldt
should inserted_at be a required argument for this then? https://github.com/python-discord/bot/blob/main/bot/exts/moderation/infraction/_utils.py#L78-L87
stable mountainBOT
bot/exts/moderation/infraction/_utils.py lines 78 to 87
async def post_infraction(
ctx: Context,
user: MemberOrUser,
infr_type: str,
reason: str,
expires_at: datetime = None,
hidden: bool = False,
active: bool = True,
dm_sent: bool = False,
) -> t.Optional[dict]:```and I'll try to trace every usage and update it to report last_applied
or it can be optional, and if None I'll calculate it in the post_infraction method
vale ibex
last_applied, not inserted, but yes
fossil veldt
um
why does this have a * argument?
https://github.com/python-discord/bot/blob/main/bot/exts/moderation/infraction/infractions.py#L286
stable mountainBOT
bot/exts/moderation/infraction/infractions.py line 286
async def note(self, ctx: Context, user: UnambiguousMemberOrUser, *, reason: t.Optional[str] = None) -> None:```is it safe to add a last_applied positional argument here before the *?
brazen charm
should that be a part of the command's interface?
tawdry vapor
why does this have a `*` argument?
https://github.com/python-discord/bot/blob/ma...
It's so the reason string doesn't need to be quoted when it has spaces
It affects the way d.py parses the arguments for the command
Yes you could add that as a positional arg but I share Numerlor's doubts.
austere hornet
Any idea why I'm getting this error when I try to start up Sir Lancebot?
I tried running pip install matplotlib cuz I didn't already have it installed but that didn't fix the error
So I'm not sure what's going on here
austere hornet
Try pulling latest changes
You mean by going to the URL of my forked repo and sync fork?
I did that and it didn't fix the error
tawdry vapor
No, that isn't what I meant. What you did was update what is on GitHub's servers. What you need to do is update your local files.
Though since your remote is now up to date you just need to use git pull to get those changes (assuming you have the main branch checked out already)
If that doesn't fix it then maybe you need to rebuild the container, though if you're using Docker Compose to run it, it should be using the source code you pulled from git already.
austere hornet
If that doesn't fix it then maybe you need to rebuild the container, though if y...
Yep, I ran docker-compose up --build and that worked. Thanks!
fossil veldt
Yes you could add that as a positional arg but I share Numerlor's doubts.
oops, did not realize that was a command 😅
got too carried away with find usages
static canyon
got too carried away with `find usages`
If you weren't aware, anything with the commands.command or commands.group decorator is a dpy command
In this case it's just command because of the import at the top of the file is
fossil veldt
yeah I'm just trying to figure out how to refactor the API for async def post_infraction and everything that uses it
dusky shoreBOT
static canyon
bot#1951 did basically what you need to do here, just with dm_sent
dusky shoreBOT
static canyon
Hopefully that helps a bit
fossil veldt
Well mainly if I add last_applied as a positional argument, it changes thec all in 7 methods in infractions.py, as well as methods in bigbrother.py
static canyon
E.g you'll need to add another parameter to post_infraction
fossil veldt
and then some of those methods use **kwargs, so it actually goes further up the chain and affects probably 30 or so methods
static canyon
and then some of those methods use `**kwargs`, so it actually goes further up th...
That seems unlikely
Lemme start up my laptop and I'll have a proper look
fossil veldt
well no it's not that much uh lemme try to count again
so just the usage of that post function is 13 methods (including 4 in tests)
static canyon
Right yeah, that seems correct
fossil veldt
but then for something like apply_ban, it's also called itself by another 10 functions:
static canyon
I'm thinkning you can just do```diff
async def post_infraction(
ctx: Context,
user: MemberOrUser,
infr_type: str,
reason: str,
expires_at: datetime = None,
hidden: bool = False,
active: bool = True,
dm_sent: bool = False,
-
last_applied: datetime = datetime.now(tz=timezone.utc)
) -> t.Optional[dict]:```
Then because it has a default you don't actually have to update the calls
You'll only need to change the edit_infraction command then, so that it updates last_applied
fossil veldt
I'm thinkning you can just do```diff
async def post_infraction(
ctx: Con...
wait can I do that actually?
I thought the value would only be called on definition?
fossil veldt
!e
from datetime import datetime
def f(val = datetime.utcnow()):
print(val)
for i in range(10):
f()
@fossil veldt :white_check_mark: Your 3.11 eval job has completed with return code 0.
001 | 2022-07-28 07:01:37.170827
002 | 2022-07-28 07:01:37.170827
003 | 2022-07-28 07:01:37.170827
004 | 2022-07-28 07:01:37.170827
005 | 2022-07-28 07:01:37.170827
006 | 2022-07-28 07:01:37.170827
007 | 2022-07-28 07:01:37.170827
008 | 2022-07-28 07:01:37.170827
009 | 2022-07-28 07:01:37.170827
010 | 2022-07-28 07:01:37.170827
But you can probably do a partial or something
fossil veldt
I could just leave it None and do an assignment to utcnow though
static canyon
Does infraction_edit call post_infraction? Cause if not we don't even need to add last_applied to the parameters
fossil veldt
uh
static canyon
We can just do last_applied = datetime.now(tz=timezone.utc) inside the function (assuming a from datetime import datetime, timezone import)
Since it's always going to be that
Yeah, it doesn't call it
fossil veldt
We can just do `last_applied = datetime.now(tz=timezone.utc)` inside the functio...
well the idea is that the mute/ban infraction functions would now parse DeltaDuration instead of Duration currently. And then we calculate the old Duration based on utcnow() + DeltaDuration, and then send both the old Duration + the utcnow() we calculated. So the human-time parser has 2 values that definitely delta to the original delta
static canyon
the mute/ban infraction functions would now parse DeltaDuration instead of Duration currently.
So this would be a change to the commands' duration typehint?
static canyon
So Duration is the actual end point (e.g. 2022-07-28 10:30:00), and not the duration (e.g. 10 min)
fossil veldt
apparently
which is kind of weird but uh
and DurationDelta would be a real duration
surreal veldt
Is there a way to delete this from my repos? (I forked it)
static canyon
So `Duration` is the actual end point (e.g. 2022-07-28 10:30:00), and not the du...
Lemme take a look at the converters rq
class Duration(DurationDelta):
"""Convert duration strings into UTC datetime.datetime objects."""``````py
class DurationDelta(Converter):
"""Convert duration strings into dateutil.relativedelta.relativedelta objects."""```Yeah, it seems that way
fossil veldt
yeah the Duration does return now + delta
static canyon
So we're instead gonna use DurationDelta, send that to Discord, and then send DurationDelta + utcnow() to the API as the expiry?
fossil veldt
I mean honestly, I think if we just put the last_applied in the post function as datetime.utcnow() it probably won't be that bad
it's not exact as some amount of microseconds / milliseconds will have passed from the command call to the post call, but is a lot better than currently (which the delta is calculated after the server response)
atomic ivy
Is there a way to delete this from my repos? (I forked it)
you can delete the fork like you delete any repo
fossil veldt
So we're instead gonna use `DurationDelta`, send that to Discord, and then send ...
yeah
fossil veldt
well actually we record time.utcnow, that will be the last_applied, and then we calculate the end time using DurationDelta + last_applied
and then those 2 things gets sent to the API
and then the user message takes those 2 values, calculates the delta again (which should be exact this time)
static canyon
Right, so the edit is also gonna have this process
In site are we now going to only store the duration, and not the end time then?
fossil veldt
So where does `last_applied` come into this?
well without it the API response only has the expiry datetime, currently the message duration text is calculated using the current utc delta after getting the response (which might be several seconds)
fossil veldt
In site are we now going to only store the duration, and not the end time then?
so the proposal is to store the initial time (last_applied) and keep the current expiry time
so the duration wouldn't be stored but rather calculated from the 2 values...?
static canyon
Would that not just have the same issue?
Because you're still calculating from timestamps which are slightly off
Off by enough to make it that it'll be 1 second off, like it currently is
fossil veldt
Because you're still calculating from timestamps which are slightly off
well they wouldn't be slightly off right? Because they are derived from the same initial place, we get the DurationDelta, and utcnow(), the sum of those is the expiry time, and utcnow() is recorded as the last_applied time
static canyon
Right, ignore that actually
I was thinking it gets calculated from the typehint, but we're using DurationDelta instead now
So how does this work in infraction_edit?
The same?
I guess it would be
fossil veldt
infraction_edit would just have to also send a last_applied I guess, but no duration
static canyon
Yeah
fossil veldt
so the new reported duration is the old expiry minus the new last_applied
static canyon
`infraction_edit` would just have to also send a `last_applied` I guess, but no ...
That's just a literal API request -- await bot.api.patch(...)
Don't think there's a function
static canyon
That's just a literal API request -- `await bot.api.patch(...)`
Since this is what I did for dm_sent
static canyon
so the new reported duration is the old expiry minus the new `last_applied`
So the database has last_applied and expiry, but NOT duration, correct?
fossil veldt
yeah
fossil veldt
And `expiry` = `last_applied + DurationDelta`?
yeah
static canyon
Yeah that sounds good
fossil veldt
the only thing is if I make last_applied a positional requirement for the post call, it affects quite a lot of functions
static canyon
(Note that it has to be now(tz=timezone.utc) and not utcnow() now though iirc)
fossil veldt
even for stuff like notes and warnings with no duration, it would still have to now report a last_applied
static canyon
the only thing is if I make `last_applied` a positional requirement for the post...
Yeah, just define it inside the function
Is there a separate created field?
fossil veldt
Yeah, just define it inside the function
so I'd send the calculated value for duration usages like mute and ban, and just leave warnings / notes to use that default value?
static canyon
I guess that's the PGSQL inserted_at (near enough)
fossil veldt
Will we be keeping that?
I think so? Last was discussing it here
#dev-contrib message
if we do remove it we'll have to make sure to send a last_edited for all new calls, since that'll be used for duration calculations
tawdry vapor
Wouldn't it make more sense (and be easier) to set this new field in the server side? Or is it ambiguous when it should be set?
static canyon
I think we are keeping inserted_at
fossil veldt
Wouldn't it make more sense (and be easier) to set this new field in the server ...
well ideally we would just send last_edited for all calls, since the server will be a bit later than the original utcnow() called in the bot
static canyon
even for stuff like notes and warnings with no duration, it would still have to ...
Surely you'd have to report last_applied to the API for all infractions?
fossil veldt
but um
fossil veldt
Surely you'd have to report `last_applied` to the API for all infractions?
yes. So if we do remove the default then all infractions will need to report last_applied
static canyon
yes. So if we do remove the default then all infractions will need to report `la...
But I'm saying don't have it as a parameter, just define it inside
fossil veldt
I was just saying less duration-sensitive stuff like warnings and notes can use the default calculated in the post infractions function, instead of being sent in
static canyon
But I'm saying don't have it as a parameter, just define it inside
Since it'll be the same value for ALL infractions -- datetime.now(tz=timezone.utc)
fossil veldt
Since it'll be the same value for ALL infractions -- `datetime.now(tz=timezone....
oh do you mean we send the DurationDelta to post infractions now?
because currently we are sending the Duration end time
static canyon
post_infraction doesn't send the message to Discord if you're thinking that it does
notify_infraction sends the DM to the user infracted
fossil veldt
yeah I knew that part, it's just, it needs to send the correct durations to the API because the notify comes from the Infraction awaited from the API response?
static canyon
And apply_infraction sends the message to pydis
tawdry vapor
well ideally we would just send `last_edited` for all calls, since the server wi...
Does that sort of inaccuracy really matter? After all, inserted_at is set server side currently. I believe the interface becomes more complicated and prone to mistakes if the value comes from the client, especially if it's not marked as a required field.
fossil veldt
so currently for a tempmute flow:
@command(aliases=["mute"])
@ensure_future_timestamp(timestamp_arg=3)
async def tempmute(
self, ctx: Context,
user: UnambiguousMemberOrUser,
duration: t.Optional[Expiry] = None,
*,
reason: t.Optional[str] = None
) -> None:
if not isinstance(user, Member):
await ctx.send(":x: The user doesn't appear to be on the server.")
return
if duration is None:
duration = await Duration().convert(ctx, "1h")
await self.apply_mute(ctx, user, reason, expires_at=duration)
async def apply_mute(self, ctx: Context, user: Member, reason: t.Optional[str],
last_applied: datetime, **kwargs) -> None:
"""Apply a mute infraction with kwargs passed to `post_infraction`."""
...
infraction = await _utils.post_infraction(ctx, user, "mute", reason, active=True, **kwargs)
async def post_infraction(
ctx: Context,
user: MemberOrUser,
infr_type: str,
reason: str,
last_applied: datetime,
expires_at: datetime = None,
hidden: bool = False,
active: bool = True,
dm_sent: bool = False,
) -> t.Optional[dict]:
currently we only get the expiry datetime as the last_applied in post_infraction
with the updated type of t.Optional[InfractionExpiry] (assuming DeltaDuration), it'd have to either get to post_infraction as a pre-calculated Duration(expiry datetime) + last_applied (current utc within tempmute), or, as the DeltaDuration, and then post_infraction calculates the 2 values.
static canyon
oh do you mean we send the `DurationDelta` to post infractions now?
Would that not automatically happen by making the command use DurationDelta?
fossil veldt
Would that not automatically happen by making the command use DurationDelta?
yes. That's a possibility. Though we might want to change the name since expires_at will be very misleading
static canyon
Yeah, I agree change the name
Which does unfortunately mean refactoring all the calls anyway
But PyCharm can do that for you
right-click -> refactor -> change signature and it'll do all calls & usages within the func
fossil veldt
that's not that bad since I need to refactor the types to the new type anyways
but there would need to be logic handing the type union since InfractionExpiry can be either the endpoint datetime or the DeltaDuration
static canyon
Where does InfractionExpiry come from?
fossil veldt
currently it's Expiry = t.Union[Duration, ISODateTime]
which both type to the same thing, expiry date time
fossil veldt
Where does `InfractionExpiry` come from?
it's a new added type for this pr
InfractionExpiry = t.Union[DurationDelta, ISODateTime]
but with this type the 2 unions are not the same time. ISODateTime is the expiry date time, DurationDelta is the delta
static canyon
Well isn't that misleading given DurationDelta wouldn't be the Expiry
I.e. InfractionDurationOrExpiry or something
And yeah, we'd then isinstance to figure out which one
fossil veldt
also I was kind of thinking if it should have infraction in the name anyways
since it's apparently used in other places
static canyon
Should or shouldn't?
fossil veldt
like superstarify uses infractions?
I guess that's fine though, it's technically an infraction still
static canyon
like `superstarify` uses infractions?
superstarify is an infraction. It prevents people from changing their nick and changes it to a celebrity (used when they have a bad nick/username)
fossil veldt
ah okay
static canyon
I don't think we need to specify it's for infractions
fossil veldt
I.e. `InfractionDurationOrExpiry` or something
I'd just call it DurationOrExpiry I guess
static canyon
Yeah
fossil veldt
that's literally what it is
static canyon
That's what I'd go for
And DurationOrExpiry > ExpiryOrDuration because of the order of the Union
fossil veldt
I guess expires_at can be duration_or_expiry then?
fossil veldt
though that's kind of a long kwarg
static canyon
It's fine
I'm sure we've got way worse lol
If you really wanted to shorten it then maybe dur_or_expiry but don't think it's worth it / necessary
fossil veldt
static canyon
Don't really like that
fossil veldt
(joke)
static canyon
Smh
fossil veldt
oh no, pycharm can't find usages
apparently every call to expires_at is through a **kwargs 😢
static canyon
lmao
Is this for post_infraction?
Cause if so then it would be thinking about it
fossil veldt
Is this for `post_infraction`?
yeah
static canyon
Yeah, you'll just have to manually change then
At least it should mostly be in the infractions.py file
fossil veldt
static canyon
By the way when this is done feel free to ping me for a review
Remember to edit the superstarify call too btw
Think that's the only one in a different file
fossil veldt
so currently I have these changes in the post_infractions body
+ current_time = datetime.utcnow()
payload = {
"actor": ctx.author.id, # Don't use ctx.message.author; antispam only patches ctx.author.
"hidden": hidden,
"reason": reason,
"type": infr_type,
"user": user.id,
"active": active,
"dm_sent": dm_sent,
+ "last_applied": current_time,
}
+ # Parse duration or expiry
+ if duration_or_expiry is not None:
+ if isinstance(duration_or_expiry, datetime):
+ payload['expires_at'] = duration_or_expiry.isoformat()
+ elif isinstance(duration_or_expiry, relativedelta):
+ payload['expires_at'] = (current_time + duration_or_expiry).isoformat()
^
static canyon
Yeah, it's weird 🤷
static canyon
so currently I have these changes in the `post_infractions` body
```diff
+ cur...
But yeah, this looks good otherwise
fossil veldt
notify_infraction uses arrow.utcnow() apparently?
fossil veldt
You can probably do an `else` though to be honest
that would avoid the delta import from dateutil yeah
static canyon
Maybe just add a comment
fossil veldt
but it could potentially match a wrong type I guess?
static canyon
else: # is a relativedelta
static canyon
but it could potentially match a wrong type I guess?
You don't really need to account for that
If anything it's probably good to not account for it because then we'll get an error when we see it (which we want because it's unexpected behaviour)
static canyon
`notify_infraction` uses `arrow.utcnow()` apparently? <:pithink:6522475599092777...
If you look, it's so that it can humanize it
fossil veldt
ah hm
static canyon
We don't need to worry about that here so datetime is the better option imo
fossil veldt
that would avoid the delta import from dateutil yeah
oh though, I guess I still have to import for the type hint
duration_or_expiry: datetime = None,
that needs to be unioned or I could import the type from converters
static canyon
that needs to be unioned or I could import the type from converters
this
And make sure to add Optional[] since you default to None
So duration_or_expiry: t.Optional[DurationOrExpiry] = None or w/e
fossil veldt
thanks for all the help btw 
static canyon
No worries 😄
It can be quite hard to get used to the bot codebase and understand the different intricacies
I probably still don't know a large amount of things
E.g. never touched help channel code
fossil veldt
wow the alias expands to this thing
static canyon
lmao