I actually think sprintf shouldn't be allowed because sprint
#dev-contrib
crude gyro
gusty sonnet
ieprint?
molten bough
the_function_formerly_known_as_print
gusty sonnet
not_actually_print_but_call_me_anyway_heres_my_number_print
mellow hare
g, that made me smile more than I thought it would
molten bough
:>
mellow hare
I think he's back to just being print though
molten bough
so 
is 0%?
mellow hare
quik maffs
woeful thorn
I can’t believe you’re mocking our blessed C prints
mellow hare
Just because it's a classic doesn't make it good
mellow hare
Understandable
woeful thorn
It’s definitely good
mellow hare
Oh god
I just realized it really is 'an user'
But I can't get myself to hear it that way
molten bough
It's okay, this is English, it doesn't have to make sense
I couldn't get over people in america saying "an hotel"
mellow hare
I've never heard that
mellow hare
an otel?
woeful thorn
Why would people in America say “an hotel”
mellow hare
Inbreeding
mellow hare
That's fair
woeful thorn
Herb is a name, urb is a plant
mellow hare
her-buh
mellow hare
It'd explain a lot
gusty sonnet
EUser
mellow hare
English is dumb
molten bough
Word.
tough imp
Is it ok to ask to be assigned to a new issue while I still have a PR awaiting review?
green oriole
I hope it is
I think I had something like 7 PRs that was awaiting for review at the same time
As long as you can handle it I think that's fine
crude gyro
@tough imp yes that's ideal.
why should you sit idle awaiting reviews
that's developer downtime
and we can't have that.
tough imp
i'm not sure if https://github.com/python-discord/bot/pull/641 should have been merged
there was still an on-going discussion regarding the asterisk use
woeful thorn
Changing our documentation style isn’t really worth holding up a completely unrelated PR
tough imp
yes, but the PR included a commit where the documentation style was adjusted to something that isn't consistent with the rest of the repo
i thought maybe that one commit is something that should be reverted (and perhaps shouldn't have been committed in the first place, thats on me)
woeful thorn
It’s ok, it can be changed
crude gyro
yeah no big deal.
tough imp
i put the backticks back in my PR for the mentions rule, hopefully thats ok 
green oriole
@tough imp hi, just wanted to tell you about a cool github feature: when you work on an issue and you made a PR to solve it, you can simply write closes #issuenumber in your PR message, and when it will be merged, the issue will be closed too https://help.github.com/en/github/managing-your-work-on-github/closing-issues-using-keywords
tough imp
yep yep thanks, i tend to forget, but i did use it in my latest PR
green oriole
Okay nice
BTW I think we should a little paragraph about it in the contributing guidelines
tough imp
ok i'm a little bit lost
i have no idea where the id is coming from
>>> from tests.helpers import MockMessage
>>>
>>> MockMessage()
<MockMessage spec='Message' id='139883050095376'>
>>> MockMessage().id
<MagicMock name='mock.id' id='139883046665168'>
>>> MockMessage(id=123)
<MockMessage spec='Message' id='139883046678288'>
>>> MockMessage(id=123).id
123
hardy gorge
The id in the repr is the regular MagicMock id, which is just the id(object) memory-address-in-int-form of CPython
tough imp
oh my god
hardy gorge
It's not showing you the id attribute of the object
tough imp
i have almost gone insane trying to figure out where thats generated / set
hardy gorge
One second, I think there's a parsimonious solution that does not require you to do anything with the id at all in your test file
It's something that probably should have been part of the mocks in the first place
tough imp
I'd be happy to do this properly, but I also wanted to keep it consitent, so when I saw that the test case that already existed doesnt use the MockMessage for a reason I thought was entirely logical, I went with it
as Scragly pointed out discord.Message doesnt actually inherit what I linked
so really I think all we need is to manually set the ids of the MockMessages so that they can be compared
they do not get compared inside the mentions.apply mind you, just in my test case, which is why I thought ok lets make this easier for myself
the tested function doesnt actually access the attrib
hardy gorge
Hmm, yes, but I think that there's a solution that would make things much easier in general
I was kinda lazy in my previous PR and did not include a __eq__ for the MockMessage
tough imp
yeah but I don't think discord.Message implements it either
tough imp
hardy gorge
Ah, you're right
Then we probably shouldn't implement something like that in the general mock type
tough imp
so what we need to do is just set the MockMessage.id attrib explicitly, right?
we can add it to __init__ I guess, I think the other MockX do it
hardy gorge
Hmm, yeah, but it's not necessary
You can always pass an id keyword to the constructor
The other ones have it because the __eq__ and __hash__ they implement rely on that id attribute being there and it returning an int instead of a MagicMock
tough imp
fair enough, should I then adjust the other test cases as well, so that they all use MockMessage?
I think that would be logical to do
hardy gorge
That's a good idea. I'm sorry for not reviewing this earlier; work has been way too busy in the last two weeks
hardy gorge
sure
tough imp
in your comment on github you mention that the reason why we use MockMessage is that if the tested method attempts to access an attrib that no longer exists (due to a change in d.py), the test cases will fail
but surely if i'm setting e.g. author explicitly in my test case
then the mock object will have that attrib, regardless of whether the actual d.py objects do or not, right?
hardy gorge
Yes, it will. I actually think we should switch to more strict version of mocking, which also prevents setting attributes that don't exist on the actual thing
tough imp
for example, if d.py Message objects no longer carry an author attrib, but we manually pass it, the test case will be able to access it
hardy gorge
So, I'll look into that later
The reason I did not is because we have custom attributes on Bot, but there's a way of assigning them that still works, so that's a shitty reason
And we're going to create custom Bot subclass anyway, so it will be entirely moot in the future
tough imp
fair enough, not trying to criticise, just wondering because I'm super new to mocking
thanks a lot for the feedback by the way 
hardy gorge
I'm learning "on the job" as well. The more I read the docs and the CPython source code, the more things I see that I should have done slightly differently
I totally missed this when I started:
spec_set: A stricter variant of spec. If used, attempting to set or get an attribute on the mock that isn’t on the object passed as spec_set will raise an AttributeError.
tough imp
oh nice
hardy gorge
Basically, what I hope is that our test suite will help us when we're upgrading either Python or d.py in the future
When we did the Django migration, we spend multiple days full time with multiple people testing literally everything to make sure it still worked
After that, I upgraded to d.py 1.2.3 from 1.0.0a (rewrite) and basically had to test a lot again (although, luckily, not everything), since there were just enough attribute name changes to easily miss them in the source code
Luckily, PyCharm helps, since it actually lints for non-existing attributes (Python itself will only complain when you actually run the line, because of late binding)
tough imp
yes yes
I'm happy to help make this more robust and elegant so that we're ready for changes like that
tough imp
ideally I think this should pass though
>>> from tests.helpers import MockMessage
>>>
>>> MockMessage(id=1) == MockMessage(id=1)
False
it looks like MagicMock.__eq__ does an identity check by default
woeful thorn
What are we doing that requires equality comparison of Message objects?
woeful thorn
That doesn’t really answer the question, it what scenario do we want to find ourselves comparing Message objects directly rather than their contents
tough imp
if the rule gets triggered, it returns a tuple containing the relevant messages that caused it to fire
within the test case, we're checking if that tuple matches the expected output
in the original solution, I simply used a custom namedtuple("FakeMessage", [author, mentions]), obviously the comparison here is trivial
but as per Ves' suggestion I'm now using tests.helpers.MockMessage
brazen charm
anyone who could take a look at https://github.com/python-discord/bot/pull/546 ?
glass pecan
sure, i'll check it out shortly
glass pecan
@brazen charm at L308:
if last_paragraph_end == -1:
last_paragraph_end = 1000
description = description[:last_paragraph_end]
wouldn't this be the same as:
description = shortened[:last_paragraph_end]
oh right, guess I was too focused on the annoying sentinel for searches
glass pecan
sentinal?
brazen charm
the -1 return when nothing is found instead of something like None
brazen charm
messed up my searching for the descriptions because I thought it was working but it was just returning -1s and shortening
tough imp
is https://github.com/python-discord/bot/pull/655 looking better now? i'd like to go and apply the same methodology with MockMessage to the other unit tests, just wondering if it's moving in the right direction
hardy gorge
I can look at it tomorrow
tough imp
ok thanks
molten bough
Do you guys have like a priority list for what to deal with next?
hardy gorge
Right, looks pretty good to me, @tough imp
I'll look into it in more detail tomorrow, as I'm kinda planning to work on the helpers for a bit tomorrow
tough imp
alright thank you
hardy gorge
Priority list, hmm, that's a good question
glass pecan
@brazen charm adding the inv difference was a great idea to the reload command. But with the cases of removal, it's not clear it's being removed but instead it's just like a list without context
molten bough
Could highlight it?
glass pecan
Makes it seem like it's just saying urllib3 was refreshed
molten bough
- urllib3
+ discord.py
yeah
glass pecan
that's the suggestion
molten bough
I think diff has another one if you want to also show just a refresh
can't remember what it is though
glass pecan
only the removals and additions are shown
brazen charm
what's the syntax highlight on that one?
glass pecan
diff
brazen charm
searched a bit but didn't come up with anything so just went with the inline to keep thjem aligned
glass pecan
makes sense for alignment. but might as well go full hog if you're showing a diff
😄
holy i forget the emojis are huge now
glass pecan
@brazen charm once that parts adjusted i'm thinking it's ok to approve personally.
so you'll only need another review for it to be merged
brazen charm
I wasn't sure about the return [], "" when getting symbol info for modules
glass pecan
seems fine to me
it matches the return types, it's consistent to how it's processed and it correctly represents the data
others might have a different opinion but you'll have to see at the time of a second review
brazen charm
oh, and how is the error handling and logging for fetching inventories? completely wasn't sure what to do there for async and the logging levels
brazen charm
thanks for looking over it
hardy gorge
What is NO_OVERRIDE_GROUPS and why are these added?
I read something about it here:
if (group_name in NO_OVERRIDE_GROUPS
# check if any package from `NO_OVERRIDE_PACKAGES`
# is in base URL of the symbol that would be overridden
or any(package in self.inventories[symbol].split("/", 3)[2]
for package in NO_OVERRIDE_PACKAGES)):
But, it's not exactly clear to me what it all does
brazen charm
the inventories group things in some way (not exactly sure how) and some are things we do not want to be overriding symbols that already exist for example pdbcommands are here https://docs.python.org/3/library/pdb.html, terms are in the glossary and scattered over docs etc.
so if a symbol has that group, it won't rewrite an already existing symbol; should I put the reasoning in a comment there?
glass pecan
I need to sleep, so I'll be back after to recheck anything. 
molten bough
Just green now I guess
patent pivot
Index: languages/ini.js
===================================================================
--- languages/ini.js (revision 199)
+++ languages/ini.js (revision 200)
@@ -1,8 +1,7 @@
hljs.LANGUAGES.ini =
{
case_insensitive: true,
- defaultMode:
- {
+ defaultMode: {
contains: ['comment', 'title', 'setting'],
illegal: '[^\\s]'
},
*** /path/to/original timestamp
--- /path/to/new timestamp
***************
*** 1,3 ****
--- 1,9 ----
+ This is an important
+ notice! It should
+ therefore be located at
+ the beginning of this
+ document!
! compress the size of the
! changes.
It is important to spell
@@ e @@
haha
patent pivot
interesting
molten bough
I guess it actually parses it now
hardy gorge
Where do you see that?
glass pecan
molten bough
Did you merge in master?
glass pecan
nothing is merged yet
molten bough
No, the other way around
hardy gorge
Hmm, I'm not seeing anything similar to that. Interesting
glass pecan
but it's like azure isn't doing it's thing
ves, it's in Checks tab
you can also go to bottom of normal view too and it'll show nothing in status
super useful
molten bough
I guess see if Azure is running CI
molten bough
Yep, that'd do it
glass pecan
yeah azure is definitely going wacko atm
i can't sign in
lol
it keeps going through an auth loop of signing in, signing out, asking to log in
good times
azure status page shows no issues though
molten bough
Lemme remote my server and I'll check
molten bough
Yep I'm getting the same thing
glass pecan
azure portal works
but azure devops doesn't
guess we'll just have to wait for things to be fixed
i'm sure they're already being screamed at by 52 thousand people
glass pecan
hahahaha
molten bough
Haha
glass pecan
i bet twitter is going then
molten bough
Twitter
molten bough
hmm, okay
molten bough
FF here
glass pecan
wonder if ves signed in recently so has a valid cookie still
hardy gorge
I'm on a different PC
not my own
Just signed in with the whole process (including 2fa)
glass pecan
huh
molten bough
That queued build doesn't show in the build list
hardy gorge
It doesn't do anything
glass pecan
probs being served by a different front end server so you logged in fine
hardy gorge
So, I guess something is broken
molten bough
yeah, must be I guess
glass pecan
i wouldn't trust anything atm then
let em fix it
a bit annoying, i really wanted to get that settings page
oh btw
we should probs add another item in the menu for admin if they're detected as allowed to access admin
wonder if that's painful to do
molten bough
Oh that's easy
glass pecan
nice
hardy gorge
yeah, it's finally building now
glass pecan
silly azure
molten bough
{% if user.is_staff %}
<b-navbar-item {% if current_item == "admin" %}active="true"{% endif %} href="{% url "admin:index" %}">
<span class="icon">
<i class="{{ icon_weight }} fa-cogs"></i>
</span>
<span>
Admin
</span>
</b-navbar-item>
{% endif %}
I mean that's a vue component but you get the idea
they want me to login in private browsing mode
glass pecan
well i guess that prevents old cookie conflicts
but like
nyeh
signed in now
woo
ok so they fixed whatever it was i think
glass pecan
thanks for poking them though. seems we only had to deal with the tail end of it thankfully
glass pecan
mm
molten bough
Vue/buefy is great by the way
if the site ever needs something highly interactive then I recommend
glass pecan
i dont mind the look of it
molten bough
it's basically as simple as you could want from a system like it, with the option of going complex later if that's what you want
Also woo merged \o/
glass pecan
ye
master build might be a bit
seems there's a bit of a queue to wait for vms
lol
molten bough
haha, figures I guess
glass pecan
molten bough
Vue basically has a templating system
You don't have to use it, but it's there
It does simplify a lot of things
glass pecan
darkmode inspector on chrome, that's new
glass pecan
lol gdude that's so backhanded
glass pecan
the users are fighting the good fight, so we'll see what happens
molten bough
That's true
glass pecan
regardless, i'm moving to a pihole
molten bough
and vivaldi aren't going to bring the breaks over
do yourself a favour and move to adguard instead
it's like pi-hole but so much better
glass pecan
for some reason i've never had a good impression of adguard
molten bough
I used to use pi-hole until I realised that half of the stuff I needed was a manual command-line job
or just not possible
glass pecan
g-hole
molten bough
haha
molten bough
Yep
glass pecan
wonder if that involves a delay
molten bough
Not in my experience
glass pecan
is it done serverside
glass pecan
hmm
molten bough
There are basically two ways you can do this
The first way, which is the default, is to have this markup as part of your actual page template, so it's on the actual page loaded by the browser
and then you tell Vue which element is the root of your vue app and it parses it like a template
The second way is to go full on Vue router mode and have it handle everything, there's also a Vue plugin that can load templates from URLs
molten bough
You can technically do it on the server, but it requires node
glass pecan
sure it's easier on the dev but it's offloading work to the client machine in return which feels shitty
molten bough
Well no, it is necessary given what vue actually gives you
there has to be some rendering on the client
like for example
<div id="app">
{{ message }}
</div>
var app = new Vue({
el: '#app',
data: {
message: 'Hello Vue!'
}
})
Now we can modify app.message and it'll update on the page in real-time
glass pecan
for now i'm ending against it as it's going against the style i've learnt since before XHTML, which is to keep client-side stuff as simple to render as possible for the same ux.
it looks nice from a dev pov
but it's just kinda squirmy in my head lol
the site is updated, which is nice
thanks for your work on that
i wanted to send a screenshot but discord isn't letting me
¯_(ツ)_/¯
brazen charm
Scargly: I changed the behaviour when the paragraph couldn't be found on module docs because I realised it wasn't really working as intended. In my tests the -1 check caught it and it just sliced it at a 1000 and I guess for you it used -1 for the whole and that went to the embed.
Just so the change isn't unexplained to you after you suggested something else
glass pecan
@molten bough the github name isn't showing on the connection card
glass pecan
ignore if it says the app is from scragly. it's in the process of transferring to org lol
molten bough
Shows for me
glass pecan
hmmmm
glass pecan
whats the difference
glass pecan
i'm kinda setup for a different thing atm. if you're stuck atm there's no stress, we'll try fix it later on
molten bough
No worries, I am a bit stuck but I won't be later
stable mountainBOT
molten bough
!remind 5h github username on connection card
stable mountainBOT
molten bough
womp
stable mountainBOT
glass pecan
lol
molten bough
:>
glass pecan
do you reckon it's simple to add a way to have the profile settings show based on a hash link
molten bough
you mean like, an anchor url? Like url#whatever
glass pecan
yep
molten bough
It should be fairly accessible with JS, yeah
glass pecan
if we made the url change to that when viewing the dialog, and had people who navigate direct to urls with it show the dialog, it could probably help when directing people
molten bough
That's not a bad idea
glass pecan
i'm sure we'll think of other small bits and pieces over time as we use it
molten bough
Yeah, I think so
glass pecan
so we don't use the email right
molten bough
We don't, but there isn't a way to not request it, I don't think
glass pecan
but the discord oauth scope shows email still, which i assume is kinda coded into the provider?
yeah ok
molten bough
Right, but that also includes the user info, doesn't it?
glass pecan
username and id, yep
glass pecan
atm it's email+identify in scopes
if it's a pain, then probs not worth worrying about. but if it's something as simple as maybe subclassing it and adjusting an attribute, that would be noice
but i wasn't sure
i haven't looked into providers properly yet
molten bough
I couldn't find an obvious way to do it when I was working with it before
glass pecan
righto
molten bough
My solution patches all of the providers, though, so even if they do request it, we never get it
(by overriding a method in the base class that's designed to be overridden)
glass pecan
yeah gotcha. we can look at it laters, it's minor
molten bough
Fair, fair
I like the idea of being able to pop up the modal
It might be nice to do that on account creation but I don't think we have a way to detect it
molten bough
it also occurred to me that there's no close button on the modal
glass pecan
almost tempted to map role colours
yeah no close but it goes away on clicking anywhere else
molten bough
Yeah, but it's probably a good idea to have one with an aria role set, for screen readers
glass pecan
mm
molten bough
Could just have it on the right at the bottom
molten bough
:>
glass pecan
it's got nothing atm, so it suits
molten bough
Yeah
Also you should totally map the role colours
An actual legit use for inline styles
haha
You can invert the text colour with CSS as well
color: <whatever>;
-webkit-filter: invert(100%);
filter: invert(100%);
just with an inner span
glass pecan
yes, i was thinking the role colours would be cool
i would like to see my admin view improvements PR reviewed sometime soon too so if we need to do more view stuff it's built up on that
molten bough
Alright
Thanks for doing the review btw
looks at watch
Three weeks is pretty good :>
glass pecan
we're all kinda burnt out still from the previous stuff, so things will take time for a bit while we recover
green oriole
The account setting button doesn't work very well on Android Chrome, it doesn't close the navigation bar, so you completely stuck because you can't close anything
brazen charm
Are there any changes on the site that change the bot interaction btw? Just have it cloned and wondering if I should update it
green oriole
You need to always put site up-to-date when you pull bot or you are going to run into rejected field issues @brazen charm
@molten bough well, it doesn't :p
brazen charm
Didn't have any problems so far
And the site is going to be older than a month now :D
molten bough
Added a ticket https://github.com/python-discord/site/issues/307
green oriole
Did you pulled also the bot? @brazen charm because some feature are probably broken like the reminder cog
glass pecan
thanks gdude
brazen charm
I have the cogs I dont work on disabled Co could br that
stable mountainBOT
@glass pecan
It has arrived!
molten bough
haha, go bed
stable mountainBOT
@glass pecan
It has arrived!
austere veldt
Hi I am working on this task https://github.com/python-discord/bot/issues/639
I have few questions regarding this, and I have posted them as comment on the issue. Can anyone help me with that?
green oriole
@crude gyro we need you please :)
crude gyro
sorry, afk
hardy gorge
What's going on?
green oriole
@austere veldt have questions about https://github.com/python-discord/bot/issues/639
hardy gorge
Okay
green oriole
Hey @hardy gorge I have written some more tests for https://github.com/python-discord/site/pull/300, can you just give it a quick look in case there is any missing tests?
green oriole
Anyway, I can either fix this issue https://github.com/python-discord/bot/issues/551 or try to setup WSL2 docker, what do you want me to do first?
Well, what is the most urgent for you?
mellow hare
I'd personally say the docker thing might be slightly more urgent, as not having a for sure way to get docker running on Win 10 home systems could discourage potential contribs
But that's just lil ol me
green oriole
Docker Desktop requires Windows 10 Pro or Enterprise version 15063 to run.
...
I...
just want to used WSL2 because it doesn't require Pro....
Well..
Not an option anymore?
green oriole
Yeah I know
I did it
And it says Docker Desktop requires Windows 10 Pro or Enterprise version 15063 to run.
You need the tech preview
green oriole
The WSL2 setup needs to install the tech preview, but the install refuse to do it
mellow hare
Hmm
green oriole
You now understand why I dislike docker
hardy gorge
Which installer did you download for Docker?
The one from that page ELA linked?
Or the normal one?
Only the one from the page linked above is meant to be used with WSL2
Not the regular one (which has the requirements you quoted)
mellow hare
Install Windows 10 Insider Preview build 18932 or later.
Yeah that's step one in the prereqs
molten bough
Akarys already said they're on an IP build back when this whole docker discussion started
mellow hare
Right, but is it up to that date
molten bough
@green oriole you on fast ring or slow ring?
mellow hare
Or build
mellow hare
And you enabled the WSL 2 featuer?
mellow hare
Just looking through the steps
green oriole
green oriole
Seems like this is a docker installer related issue https://github.com/docker/for-win/issues/4586
hardy gorge
@green oriole Are you here?
green oriole
I am
hardy gorge
I'm looking at your tests now; they are looking good
A couple of small things:
def test_returns_400_on_negative_id_or_channel_id(self):
url = reverse('bot:offensivemessage-list', host='api')
delete_at = datetime.datetime.now() + datetime.timedelta(days=1)
data = {
'id': '-602951077675139072',
'channel_id': '291284109232308226',
'delete_date': delete_at.isoformat()[:-1]
}
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {
'id': ['Ensure this value is greater than or equal to 0.']
})
data = {
'id': '602951077675139072',
'channel_id': '-291284109232308226',
'delete_date': delete_at.isoformat()[:-1]
}
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {
'channel_id': ['Ensure this value is greater than or equal to 0.']
})
If you look at this test, it actually tests two things: First a test for a negative id and the second time for the negative channel id
By testing it like this, if the first fails, we will never run the second one
It's also the case that you basically have the same code twice, copy-pasted
We can solve that by using an iteration and self.subTest
Let me rewrite it for to show what I mean
This is a bit of a naïve solution, the main data dict could probably be factored out, but:
def test_returns_400_on_negative_id_or_channel_id(self):
delete_at = datetime.datetime.now() + datetime.timedelta(days=1)
default_data = {
'id': '602951077675139072',
'channel_id': '291284109232308226',
'delete_date': delete_at.isoformat()[:-1]
}
cases = (
('id', '-602951077675139072')
('channel_id', '-291284109232308226')
)
url = reverse('bot:offensivemessage-list', host='api')
for field, invalid_value in cases:
with self.subTest(field=field, invalid_value=invalid_value):
data = collections.ChainMap({field: invalid_value}, default_data)
response = self.client.post(url, data=data)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json(), {
field: ['Ensure this value is greater than or equal to 0.']
})
Hmm I see..
hardy gorge
Admittedly, there are large parts of our current test suite that don't do this, but we should probably try to get into the habit for the future and refactor the old stuff at some point as well
One other thing, you occasionally use datetime.datetime.now(tz=datetime.timezone.utc) and occasionally use utcnow and later a replace
e.g,
delete_at = datetime.datetime.now(tz=datetime.timezone.utc) + datetime.timedelta(days=1)
cls.valid_offensive_message = OffensiveMessage.objects.create(
id=602951077675139072,
channel_id=291284109232308226,
delete_date=delete_at.isoformat()
)
vs
delete_at = datetime.datetime.now() + datetime.timedelta(days=1)
data = {
'id': '602951077675139072',
'channel_id': '291284109232308226',
'delete_date': delete_at.isoformat()[:-1]
}
aware_delete_at = delete_at.replace(tzinfo=datetime.timezone.utc)
wait wrong example
green oriole
Yeah I forced the timezone on direct object creation or I would get a warning during the test run
Oh yeah this
hardy gorge
I don't actually DRF has issues with the timezone suffix (z) being there
Not entire sure
green oriole
I wanted to mimick the exact way the bot create the object (with a naive datetime), but the datetime stored in the db have a timezone so I had to replace it to execute the test
hardy gorge
Ah, right. That makes sense.
This test:
def test_returns_405_for_patch_and_put_requests(self):
url = reverse(
'bot:offensivemessage-detail', host='api', args=(self.valid_offensive_message.id,)
)
response = self.client.patch(url, {})
self.assertEqual(response.status_code, 405)
response = self.client.put(url, {})
self.assertEqual(response.status_code, 405)
Also does two truly separate tests in one test (put method test and patch method test)
green oriole
Yeah I need to change that too
I will do it tomorrow it is now time to go to bed
See you and thanks for you help!
hardy gorge
Sleep well! Tests look good, I think we can wrap this up tomorrow
green oriole
Hi @hardy gorge are you here?
hardy gorge
I'm at work
green oriole
Oh okay 😄
gusty sonnet
Hmm, anyone knows how to send an image as attachment to discord via webhook? I'm reading the docs, they only say file contents - I assume I have to send byteIO?
glass pecan
yep
gusty sonnet
Ah alrighty, thank you
glass pecan
this seems to be how dpy does it:
https://github.com/Rapptz/discord.py/blob/4c3e53edf4ff90e3fcf195c429601a553d661cd8/discord/webhook.py#L119
austere veldt
Hi I have added a new cog with the name whitelist in the whitelists.py file. I have added the cog using the setup method. I restarted the docker but I cog did not get loaded. Is there any other place where I need to make change, to make it work?
woeful thorn
Did you add it to the Bot's __main__.py file? https://github.com/python-discord/bot/blob/master/bot/__main__.py
austere veldt
No, I didn't. Thanks!
austere veldt
I want to store whitelisted items for various types. There will mostly be operations to fetch a whitelist for a particualr type or update it.
Is this a good model design, keeping in mind that there can be more whitelists like this in future.
class Whitelist(ModelReprMixin, models.Model):
"""Whitelisted items for a type."""
GUILD_INVITE = "guild invite"
FILE_EXTENSION = "file extension"
CHANNEL = "channel"
ROLE = "role"
TYPE_CHOICES = (
("guild_invite", GUILD_INVITE),
("file_extension", FILE_EXTENSION),
("channel", CHANNEL),
("role", ROLE)
)
type = models.CharField(
choices=TYPE_CHOICES,
max_length=100,
help_text=(
"Type for which the whitelist is stored."
),
primary_key=True
)
whitelisted_items = pgfields.ArrayField(
models.CharField(max_length=200),
help_text=(
"List of items to be whitelisted."
)
)
@tawdry vapor do you have more edge case when you test the message_edit? I actually tested it and didnt get the result you got for some reason, but nevertheless I did some modification
I want to quick test them in my server
For some reason this is what I got when I tested with your case
tawdry vapor
I don't. That was the one and only test I did and it happened to fail
gusty sonnet
Weird indeed
tawdry vapor
Your test is not exactly the same
The original should have an exclamation at the end
tawdry vapor
Oh you may not be in that server
gusty sonnet
tawdry vapor
Well I'll run it again to make sure
gusty sonnet
I found something weird with difflib actually
I ran the same thing over and got different results somehow, in a snippet
some kind of inconsistency
Nvm it was the exclamation mark
But I see the problem now
tawdry vapor
Oh ok
gusty sonnet
Yep going with a list fixed that entirely, ty again Mark
tawdry vapor
np
gusty sonnet
Ogod I misread your case, your 2nd message does not have a message lol
But the commit fixed that as well, list does seem to be the solution
green oriole
I just created a new model, created migrations and written tests for the issue I'm working on atm, should I break them in 3 commits or I can just do a big one?
glass pecan
separate is fine
they're logically grouped in those cases
just make sure the commit names are clear on what they're doing and you'll be fine
like avoid "add tests"
lol
tough imp
would it be worth it to use helpers.MockMember to mock the MockMessage.author, if the authors get ==ed in the tested function?
tough imp
hmm, could someone check something with me please
i think there may be a bug here https://github.com/python-discord/bot/blob/c04df6472b2450d273475df94fa4917116b451e0/bot/rules/attachments.py#L10
this can cause last_message to appear twice in relevant_messages i believe
therefore potentially counting twice in the sum, should it have attachments
this gets called from here https://github.com/python-discord/bot/blob/master/bot/cogs/antispam.py#L171
other rules such as this only use last_message to grab the author reference https://github.com/python-discord/bot/blob/c04df6472b2450d273475df94fa4917116b451e0/bot/rules/mentions.py#L10
the rules are called in the same way, but there is a discrepancy in how they treat last_message
green oriole
Hmm from what I remember from my testing, the filters seems to trigger way before the limit fixed in the config
hardy gorge
That doesn't sound right.
If you can replicate it, please make an issue describing how you've done that, because we'd need to look into that.
tough imp
i'll make an issue tonight
hardy gorge
I was more talking about the problem Akarys highlighted, yours seems more clear cut given the code that's there.
I've personally not seen these miscounts with filters and some of them already have unittests that don't show them, so we need a good way of replicating the problem if it does occur
tough imp
the problem is that the unit test for that rule emulates the way the relevant messages vector is built inside the rule
i found out when i was adjusting the test case to use MockMessage
because i thought the way it counts the messages is inconsistent with how i count them in the other unit tests
when i adjusted the unit test to be consistent with mine, it wouldnt pass
so theres a difference in how the rules behave
but the problem is that i dont think theres any specification on how the rule should behave
so the unit test has to follow something
i suppose the unit test should be the specification
hardy gorge
Yes, I think that our rules should all "count the same", because, If I'm understanding your correctly, the limit set in our constants would be interpreted differently for different rules.
A limit of 6 should mean the same for every rule, in my opinion
green oriole
I'm 99% sure that burst trigger after 3 or 4 similar messages
I will do some testing
hardy gorge
It did not when I was working on the antispam cog two months ago or so
green oriole
Doesn't seems right isn't?
green oriole
Oh
hardy gorge
Try typing out 1, 2, 3, 4, 5, ... to get the burst filter
green oriole
I though attachment was the one that triggered
hardy gorge
That's the one that's triggering 1 early, going by @tough imp's findings above
green oriole
Well if there is actually an issue about that, I have an issue about adding attachments to the staff logs, maybe I could kaizen add it
tough imp
In attachments I think all that needs to be done is remove the '[last_message] +' bit (sorry am on phone)
Since last_message is recent_messages[0]
green oriole
So there is really an issue? Okay
tough imp
I can look more into it later today and check other rules
But feel free to refer to my messages above with the links to github
glass pecan
@snow zealot all our members are welcome to suggest and talk about our projects over here
all of our contributors talk here anytime so they'll be able to answer questions where necessary if you have any
if you do feel like there's any features you'd like to suggest, you're welcome to create issue tickets also on the repo or just ask about viability here
snow zealot
that's great, I never actually looked into this channel before today
alright, thanks!
glass pecan
no probs. i hope to see some activity maybe from you sometime in future, would be great to see
snow zealot
Hopefully, I'm not that smart tho but yeah 😄
glass pecan
don't have to be smart. people here often learn as they go
also often it's not about being "smart"
it's all about a willingness to learn and a bit of time and effort
you'd be surprised how simple some tasks are that we want
we have 131 issues open atm
5 issues are explicitly marked as "good first issue" so are suitable for beginners
snow zealot
you say, 'some tasks that we want', as in you say what you want from the bot and someone(anyone) comes up with code for it and submits it to y'all?
smth like that?
glass pecan
issue tickets outline ideas to contribute to the bot
some of them might be assigned
(2 of the beginner friendly ones are)
but the 3 other non-assigned issue tickets could potentially be worth looking at to see if you'd be interested at coding, creating a PR and contributing to the project
that's the search for all issue tickets that are marked as good first issues
here's an example
this is asking for someone to add a ping to existing message output
snow zealot
I see, I think I get it
glass pecan
hmm looking at the comments in there, the issue seems to be not decided on yet
which is understandable
but the task difficulty i'm sure you understand a bit now
this one is open
removing a ping from an existing output message and adding a conditional emoji instead to a different message
snow zealot
So two ways for me to contribute would be, either looking at open issues(perhaps the good first issues primarily) and see if I can code it up(or learn enough to code it up in a reasonable time), or suggest a feature.
glass pecan
yep, exactly
snow zealot
and so, I'd need to learn about coding bots on discord to be able to actually contribute code of substance
glass pecan
if you can think of a new feature, enhancement of an existing feature or you point out any existing bugs that could be fixed, that's definitely a contribution
even without coding the solution
snow zealot
hm, I never really thought of this before today but you're quite motivating to say the least
glass pecan
if you personally want to show up as a code contributor you'll want to code some form of solution
snow zealot
is there a place you started learning from, about coding bots for discord
glass pecan
there's a few different places
there's a really basic introduction to the library discord.py here:
https://gist.github.com/scragly/095b5278a354d46e86f02d643fc3d64b
glass pecan
a lot of the actual learning is done by reading the prose docs of the library that's linked in the gist
but the intro regarding basic knowledge, tips and code snippets can be helpful
snow zealot
the gist?
snow zealot
Oh right didn't know that
prose docs perhaps means
the docs mentioned in the link yeah?
glass pecan
prose docs means explanatory documentation, rather than basic api docs
you'll find both linked in that
yep
snow zealot
Right, I think I'm gonna explore that link and things inside for sometime and see what progress I make
glass pecan
everything here in the dpy docs is "prose docs"
the migration guide is only useful really if you're moving from an older lib though to the new version
if you're not familiar with dpy yet at all, just skip that (optionally)
snow zealot
yeah I've never done anything like this before so yup
glass pecan
it's kinda fun once you get into it
we also have more than just that bot
this is our season event bot
it has an even lower threshold for contributors
snow zealot
oh yeah I saw this around during the egg-dropping event thing and a few more events
glass pecan
this was specifically designed to be the introduction bot for new potential contributors
it's grown a little ofc lol
brazen charm
Seasonalbot is a lot easier to set up but there aren't as many issues for it now out of season
snow zealot
Oh I see
glass pecan
still 33 issues open, looks like about half are assigned
snow zealot
hm so the gist you linked would be a good starting point for someone with no experience yeah?
snow zealot
alright gotcha
glass pecan
i gotta definitely rewrite and expand on the guide
glass pecan
it's old and was tossed together in 20 minutes
glass pecan
it's still valid but it can be improved
i'll be moving it to the website as a wiki guide though eventually
that'll be a better place for it to live
snow zealot
right
well until now, I really had no motivation to get into this sort of thing but I kinda feel excited now, I'll definitely invest time and efforts into this
green oriole
That remind me of the beginner issue I'm working on where I needed to create a new model in the site db
glass pecan
that would be awesome if you do
we'd love to see more people jump in and contribute anyway they can
green oriole
Nice! Always more job for the staff!
glass pecan
lol akarys, got the short straw there
snow zealot
never expected testing the mute feature of bot would lead me here haha!
snow zealot
;)
green oriole
tbh I excepted it, I just don't know what it is tagged like that
snow zealot
thank you
glass pecan
don't think i'll forget either
imma wait to see at least something from you within a week!
hahaha
even if it's either a new issue ticket or you asking how to setup one of the bot projects
green oriole
What os do you run jester?
glass pecan
!remind 7d Ask 495273546864525313 wtf he's up to and if he's gonna contribute.
stable mountainBOT
glass pecan
good botto
snow zealot
@green oriole Windows 10
snow zealot
home on my pc, pro on laptop
snow zealot
is that uhh bad?
snow zealot
I can but I'd prefer to use my desktop if that's okay
green oriole
That's okay of course, just a bit more steps
glass pecan
its fine. I'll have to remember to do a win home setup guide eventually
i don't have windows home so i'll have to vm it though
snow zealot
yeah at this point I dun even know what docker and nondockers are xd so I'll read up on that first
snow zealot
mhm
glass pecan
as a taste though, think of it like tiny virtual machines
green oriole
BTW scrag I asked if you want me to write a guide for it (or at least the outline of a guide) have you decided if you want it or not?
glass pecan
we have always wanted it
so if you can write up something, that would be orgasmic
pls make a gist for it first so we can see how it reads
green oriole
Let's go then!
glass pecan
since gist is markdown it'll be super simple to move to the wiki
green oriole
Yep
snow zealot
I'm gonna go dive into the gist then and get back here if I get stuck(or google it first ofc)
glass pecan
go for it
snow zealot
;)
green oriole
We already have some nondocker instructions?
hardy gorge
There's a very brief mention of dockerless set-up in the documentation subfolder of site
glass pecan
or rather, after the docker setup guide was done but i do remember people talking through a setup
snow zealot
oh btw, is knowledge of python sufficient for this kind of thing? I haven't done JS and things like that
green oriole
@hardy gorge do you have a link please?
snow zealot
alright
green oriole
Ooh in the git thanks!
glass pecan
To run the server, run
python manage.py runserver
technically it's python manage.py run --debug now
or pipenv run start is actually better
now that the pr was merged
green oriole
Chrome just restarted and loose everything i typed
there is just a dot remaning
I’m.. trying to stay calm..
Anyway, as i was browsing through the existing docs i found some strange things
Like this sentence For any Core Developers, since you have write permissions already to the original repository, you can just create a feature branch to push your commits to instead., but any person from the staff or any contributor
hardy gorge
We changed the permissions since then
It used to be core-dev only
Now both staff members (helpers+) and contributors have write-access
So, it should be updated
brazen charm
Does that even need to be public or addressed there?
hardy gorge
It's good to have there, I think, since it's good for people to know that they may have write-permissions
green oriole
It is written in the role description page and some staff members have mentioned it a few times, so i don’t think that a confidential information
hardy gorge
If people have that, I'd prefer it if they were to make branches instead of forks, since that makes reviewing easier as well
Plus, their reviews will count
green oriole
And sedsarq didn’t knew they had write access, so i think it is worth mentioning
Well only staff reviews count
brazen charm
I mean it's not confidential but it's useless info for your average Joe
green oriole
But it is useful for staff members
past falcon
Myeah I was just following the contrib guide a bit too literally
Which said something like "fork it" at some point
green oriole
It also say you don’t need to if you are a core dev, but this note is after saying "fork it"
So yeah..
I’m writing the nondocker guide, and i need to make some steps about creating the postgres user and database, should i use the GUI or i can use command line? I think the command line would be way easier than using pgAdmin, but the other guides try yo always use GUIs
hardy gorge
I'm in favour of the command line option, since it usually reduces the number of steps required to explain it
but, I'm open to hearing opinions on that
green oriole
Yeah i had the same opinion, but since every other guides try to use GUIs as much as possible, I wanted to make sure it is fine
past falcon
I prefer GUI personally just because weird commands dont necessarily explain whats happening. But if i click on buttons like "create user", "change permissions" etc i might be able to figure it out next time
molten bough
I mean, most GUIs are just running the same commands for you anyway
Might as well figure out what it does under the hood
green oriole
I can explain what the commands does, they are just SLQ commands
But yeah sed is right
i guess i can just do both?
Like this page for example https://pythondiscord.com/pages/contributing/installing-project-dependencies/
But the GUI just create the SQL command for you..
past falcon
Thats fair. If the guide covers the conceptual stuff, thats even better
glass pecan
thanks
its a bit late for me to look over anything atm
but i'll be sure to do so tomorrow
stable mountainBOT
green oriole
Yep, i’m gonna try to write the bot guide in the meantime
hollow lichen
@green oriole Never write big text on an online-editor 😉
mellow hare
Not sure why not. The GitHub one works well enough
brazen charm
easy to lose I assume
green oriole
Yeah it is easy to lose https://discordapp.com/channels/267624335836053506/635950537262759947/644539181619609629
mellow hare
Fair enough
green oriole
Well that's the only thing I have on my iPad that more or less support markdown
Anyway
What do you guys think of my guide? Anything I can improve?
mellow hare
I'll peek at it when I can. Currently bouncing between two computers taking care of stuff
glass pecan
i always copy the text i'm working on before pressing any buttons
and the clipboard shows up to 20 items in history
so i feel pretty safe with that
green oriole
Oooh that's a good idea
tough imp
ok so it looks like the attachments rule is the only one suffering from the bug where it counts the last_message twice
now the question is how do we fix this, should I include it in my PR that was originally only to write a test case for a different rule but is now adjusting the test case for attachments, should I open a new PR for it, or @green oriole do you want to include it in your PR if you'll be having it merged soon?
green oriole
Well the PR is finished, I'm just waiting for reviews
tough imp
alright
i think i can squeeze it in mine
just going to revamp the PR so it reflects on all the changes and i think i can get it done tonight
going to include some description of why and how the rule deviates from the others
crude gyro
super fun bot command issue just created at https://github.com/python-discord/bot/issues/661 if anyone wants something cool to work on.
get it while it's hot.
this would be immediately useful to us so we'd be grateful to see it implemented.
tough imp
i'll look into how difficult it would be for me, still discovering the code base
also planning to do the automatic available server icon refreshes but been stuck on the unit tests
stable mountainBOT
@glass pecan
It has arrived!
tawdry vapor
Hi, please review my short and simple PR https://github.com/python-discord/bot/pull/619
gusty sonnet
I remember reviewing this, then I asked about logging.info() and forgot
tawdry vapor
Thanks
crude gyro
ready to merge, @tawdry vapor
not gonna merge it, though
oh wait never mind
618 is already merged
well then, I guess I can merge it
tawdry vapor
Thanks for your review
crude gyro
no probs. wasn't exactly hard to review.
brazen charm
Mind looking at #651 while you're at it?
tawdry vapor
.issue 651
dusky shoreBOT
[404] Issue/pull request not located! Please enter a valid number!
dusky shoreBOT
brazen charm
Right
tawdry vapor
We should probably whitelist this channel
I think it was but the id changed when we archived
Or the id is just wrong
One says unavailable channel
brazen charm
Should be an easy fix anyway
green oriole
The ID have probably changed, because the old channel is now archived
tawdry vapor
!remind 16h 651
stable mountainBOT
molten bough
Scragly was gonna do that, make the command work here
gusty sonnet
.issue 651 bot
dusky shoreBOT
gusty sonnet
Wait a min
brazen charm
That could be pretty cryptic reminder :P
gusty sonnet
That's from seasonal?
molten bough
The command is yeah
gusty sonnet
Hmm, I havent taken a look at seasonal bot actually, dived straight to off-season bot
green oriole
Well that’s almost the same
In term of structure
Also, if you can try to choose which channel you want the attachments to be relayed when you can, so we can get it merged, that would be nice too https://github.com/python-discord/bot/pull/630
gusty sonnet
Hmm, is this part a typo somehow
self.message_deletion_queue[message.channel.id] = DeletionContext(channel=message.channel)
self.queue_consumption_tasks = self.bot.loop.create_task(
self._process_deletion_context(message.channel.id)
)```Both of them are declared as dictionary
But the 2nd is being overwriten constantly
This is the init```py
class AntiSpam(Cog):
"""Cog that controls our anti-spam measures."""
def __init__(self, bot: Bot, validation_errors: bool) -> None:
self.bot = bot
self.validation_errors = validation_errors
role_id = AntiSpamConfig.punishment['role_id']
self.muted_role = Object(role_id)
self.expiration_date_converter = Duration()
self.message_deletion_queue = dict()
self.queue_consumption_tasks = dict()
self.bot.loop.create_task(self.alert_on_validation_error())```
That's the bigger problem
They are supposed to be dict
From my guess they are to create a task by channel
gusty sonnet
I know, this is not related to your PR yet
It is by channel id
self._process_deletion_context(message.channel.id)```I was reviewing, and I was going to ask you why you deleted the asyncio.sleep(10)
Then found out that this part is kinda weird
Right above,py self.message_deletion_queue[message.channel.id] = ...
Below, it is being used as a normal assignment
self.queue_consumption_tasks = ...```Both are dicts
gusty sonnet
I know, that's why I said it is very weird
@hardy gorge Do you happen to know who designed this part?
gusty sonnet
That's true
Ah I got the right person, it is Ves
Anyway is there a reason you deleted the asyncio.sleep(10) @green oriole
Do you need the data immediately?
It looks like it is used to delay the deletion, so the messages stay for about 10s before it vaporates
green oriole
Yep i need the message to not be deleted immediatly so i can re-upload the attachment
gusty sonnet
Hmm, this is to delay deletion process actually
So wouldnt removing it start the deletion right away?
green oriole
I had trouble understanding this part, but it doesn’t wait 10 seconds at all
From my testing deleting it fixed the problem
Oh yeah now i remember
gusty sonnet
Yep
Because of the check
I see that we need confirmation from ves first to review that part
green oriole
the deletion context is the process that upload the messages, not the one that delete them
gusty sonnet
if context_id not in self.message_deletion_queue:```It will always return False in this part
Eitherway the sleeping shouldnt have anything to do with this part
green oriole
I got some 404 errors with the sleep and i haven’t got any without it
gusty sonnet
Your reupload part looks good
Did you consider context manager for it?
Something like py for attachment in message.attachments: with ByteIO() as buffer: await attachment.save(buffer, use_cached=True) reupload = await channel.send(file=discord.File(buffer, filename=attachment.filename)) out.append(reupload.attachments[0].url)
To ensure the closing of them buffer
green oriole
I didn’t knew you could do that
gusty sonnet
Hmm, other parts looks ok, Imma comment on that part and asked Ves
hardy gorge
I'm on mobile, but my guess is you mean the delayed consumption part?
gusty sonnet
Yes, and the overwrite part as wel
I see that you wanted it to be a dict
But it was an overwrite
hardy gorge
I'll get back to you on that. I have to view the code
Delayed consumption is so that if the spamming continues, we have a single event, instead of multiple events we had in the past.
gusty sonnet
I had a feeling it is to prevent multiple pilling events
Just that since it is being overwritten, it is no longer a dictionary so the check will probably crash
hardy gorge
Uhm, I can't see the code, but antispam has worked since I added that part two months ago
hardy gorge
Can't see the changes now, on mobile, but don't remove the delay
It's there for a reason
gusty sonnet
Hmm, weird
green oriole
But I need to upload the context before the messages got deleted
gusty sonnet
Yeah that part I understand
Anyway, here, in the Cog, you have these in __init__ py self.message_deletion_queue = dict() self.queue_consumption_tasks = dict()
So these two are dict, yes?
hardy gorge
The delay is there to make sure that if more messages trigger rules during a single spamming event, they end up in the same context, instead of multiple contexts. That happened a lot in the past and was very annoying.
gusty sonnet
Inside the for loop of the on_message event, the first is used like this
self.message_deletion_queue[message.channel.id] = DeletionContext(channel=message.channel)```which is a perfect example of a dictContext for every channel id
The second however, is overwritten
self.queue_consumption_tasks = self.bot.loop.create_task(
self._process_deletion_context(message.channel.id)
)```