yeah that does not sound safe.
#dev-contrib
crude gyro
mellow hare
One sec
patent pivot
celest charm
this is a less embarassing example haha
patent pivot
now, it's not necessarily unsafe and I haven't been able to break it myself, but as xx says, user input to a state machine is not a fun game to play
brazen charm
The implementation I proposed is easy to do with no changes to snekbox itself, can just send the code over and print JSON or something to get info back out easily
mellow hare
patent pivot
we used the regex module which does prevent catastrophic backtracking & has a timeout, but I don't think we should hedge our bets on it
so the proposal now is to move it to snekbox and write a template which just runs a regex parse in python eval
green oriole
I'd be surprised of the stdlib implementation wasn't safe tbh
molten bough
It isn't
patent pivot
and I'm proposing we ditch the regex module and switch back to re, because I want to be able to catastrophically backtrack in snekbox
mellow hare
Would be moot if it is or isn't if it's run inside snekbox
patent pivot
@green oriole it isn't, look at the PR
import re
pattern = r"(\w+)\*([\w\s]+)*/$"
re_compiled = re.compile(pattern)
results = re_compiled.search('COPRO*HORIZON 2000 HOR')
print(results.groups())
infinite loop, 100% cpu
green oriole
Well, outside of backtracking
patent pivot
right, but backtracking is unsafe lol
green oriole
I mean, a timeout is enough
mellow hare
Sure but why take that risk
patent pivot
you were talking about re, which does not have a timeout
mellow hare
Especially when we have a way to properly contain it (through snekbox)
patent pivot
regex is safer, yes, it has a timeout, but I'm still not 100% trusting
green oriole
We can cancel asyncio coroutine
celest charm
Hm, it would actually make sense to backtrack catastrophically in the command. Because using regex will give unexpected surprises when the user runs the regexp with re.
clever wraith
Especially when we have a way to properly contain it
@mellow hare still can't react, but
green oriole
But yeah, I do think snekbox is the solution
patent pivot
yeah, I really want to be able to break the regex
crude gyro
okay, so. I'm not comfortable with us merging any public function that allows unsandboxed evaluation of expressions, I don't care what package we use for it.
patent pivot
I want it to crash and timeout
cold moon
Is there any way to isolate this totally inside bot?
crude gyro
if it's staff only, I'd be less fuzzed.
clever wraith
we have the technology, might as well use it
patent pivot
we want to be able to demonstrate to users that specifically crafted regular expressions break completely
@cold moon not really
celest charm
so, moving to 🐍 📫 then
mellow hare
we have the technology
We can rebuild him....
green oriole
Maybe we could take that as an opportunity to make generic snekbox templates, like the tag system
crude gyro
snekbox sounds okay, if snekbox dies there's no funeral
patent pivot
yep
snekbox has all the relevant limitations on there and a max of 2 concurrent evals
clever wraith
@celest charm idunno if you're just going to be passing it to the eval endpoint or what, but if you're adding a separate endpoint for snekbox, I'm also implementing Unix command execution as another one
green oriole
2 concurrent evals per what?
patent pivot
@clever wraith no extra executor, just a template ran in the python executor
@green oriole globally
clever wraith
you can check out my branch if you want to see how i've done it (so far) for some resemblence of consistency, if that makes sense
ah, sure
patent pivot
snekbox will not run more than two evals at once
crude gyro
the unix command execution is still on? there's been significant concerns about it.
clever wraith
has there?
patent pivot
oh?
crude gyro
yes but I'm not sure if they've been voiced in here.
green oriole
NsJail is rock solid though
clever wraith
well, i'd like to hear them
patent pivot
yeah, i wasn't aware of it either, should probably go on the PR https://github.com/python-discord/snekbox/pull/71
molten bough
I feel like I'd want snekbox on another machine if it had command execution, even with nsjail
It's a tricky vector
clever wraith
nsjail chroot + docker, mind you
green oriole
It is in another container already, we have two isolations
brazen charm
I don't think an additional endpoint is worth the time when the python code eval is there and reflects re directly.
Can construct some sort of "API" in the template it have it future proof without changes to the template
crude gyro
the overall concern is "how can we possibly make this secure, and is that worth it when the feature is baarely in our scope anyway?"
molten bough
There are docker escapes though
green oriole
But no NsJail escape
molten bough
Not yet.
celest charm
Most people can execute Unix commands on their devices now, right?
crude gyro
well we don't particularly want people breaking the sandboxes all the time, either
mellow hare
@molten bough We could be pulling our hair out worrying about the future possibilities
crude gyro
even if they can't escape
mellow hare
But I'm starting to lean towards lemon's side
clever wraith
of course I think NsJail can also be escaped, and docker from that. Is someone who's capable of doing that going to be time spending time breaking a bot on Discord? @molten bough
mellow hare
There are plenty of resources that can help folks with regex.
patent pivot
@mellow hare this isn't about regex now, this is about the UNIX command executor
molten bough
I mean I don't see why someone with the knowledge wouldn't try, a lot of people had fun trying to before haha
mellow hare
My mistake
patent pivot
regex is safe since it has the same sandboxing as current python eval, unix would allow for shell commands to be ran
clever wraith
there's still a hard pid, memory, time limit
mellow hare
Ah, okay, I'm on the same page now. Why was there a push for this?
clever wraith
it's not "unrestricted linux box" per se
mellow hare
It really doesn't seem in scope
clever wraith
but if people are concerned, I'll just drop it
crude gyro
@clever wraith wanted to do it and it seems like a neat feature.
but there are concerns and I do think they are valid.
not to be a party pooper
mellow hare
@clever wraith It's not a criticism towards you or the idea, just might not be the right place for it
molten bough
It definitely sounds useful, it just feels tricky to ensure
clever wraith
no, i understand the concerns, perfectly valid
crude gyro
but the consequences if someone did break out? it could destroy us.
clever wraith
I'll just close the PR, haven't had time to work on it in a month anyways haha
crude gyro
maybe if, like gdude said, we had the whole sandbox on a second server
but that's not in our infrastructure budget atm
celest charm
Maybe we could allow that for staff members.
molten bough
You could do what some IRC bots do
mellow hare
Do we still have bravo?
brazen charm
I don't think that would be something that's particularly used over here except a few of the topical channels
molten bough
Run stuff on free Google cloud instances
dull jetty
"but the consequences if someone did break out? it could destroy us." my god that's dramatic
patent pivot
no, we don't have bravo
mellow hare
Gotcha
dull jetty
yeah i could just spin up a super limited ec2 instance which will accept fed in commands and has zero IAM role perms
so you kind of just let it die if it dies
or at least it's ok if that happens
patent pivot
I don't really want this to introduce more infrastructure changes
crude gyro
we have backups and stuff, it's not a big deal. I'm worried about data protection.
dull jetty
assuming no one manages to somehow XSS-style attack back into whatever the bot prints
clever wraith
I've closed the PR, too much hassle for too little benefit as people said
clever wraith
@dull jetty the "xss" side of things is already taken care of by the same mechanism as !e
clever wraith
@crude gyro time which i have enjoyed spending is not time wasted :)
crude gyro
works fine.
molten bough
Yeah I guess that's true
celest charm
And if they decide to shut down their API again, we're in an awkward position
molten bough
They didn't actually shut it down, you just can't sign up for it
They're remaking it
celest charm
oh
crude gyro
or at least it's ok if that happens
@dull jetty
it is fine if the server dies, we will spin up a new one in no time. but if someone had actual access to it, we could be in trouble.
I don't want to go into details on why.
patent pivot
@crude gyro that wasn't the proposal from seagull, seagull was proposing additional infra to handle evaluations away from our server
patent pivot
but again on the infra point, we don't have a big devops team, we'd rather keep things sweet and simple
crude gyro
yep yep.
patent pivot
on the regex point, moving it to the python evaluator sounds like a good plan
crude gyro
yep. works for me.
celest charm
Would it be horrendous to use the Snekbox cog directly? Or do I just copy the interaction?
brazen charm
I don't think it would be bad assuming it would be beneficial
patent pivot
using snekbox directly doesn't sound unpleasant, but maybe it should be considered to move utilities out into a snekbox utils file
brazen charm
though there's no reason to go through it whole since it has a lot of sanitization
green oriole
There's only one function you need
upload_code() iirc
All the rest is just formatting
celest charm
yes
async def post_eval(self, code: str) -> dict:
"""Send a POST request to the Snekbox API to evaluate code and return the results."""
url = URLs.snekbox_eval_api
data = {"input": code}
async with self.bot.http_session.post(url, json=data, raise_for_status=True) as resp:
return await resp.json()
This one yeah
celest charm
Maybe I can just copy it for now, and we'll move the snekbox utils into a separate file.
Or should I reference the Snekbox cog?
brazen charm
With it being that short you could probably do the request yourself and then do some processing in one func
celest charm
true
brazen charm
Since you can specify what exactly goes out to stdout for working with the data
green oriole
You should keep them separate, for the sake of writing tests
woeful thorn
That doesn’t make any sense
It’s already being tested
You don’t need to rewrite the function to test it again for a marginally different use case
green oriole
I don't think the regex cog is being tested, it doesn't exist yet
woeful thorn
Yeah, thanks
green oriole
Writing tests for snekbox was really easy thanks to those short functions
woeful thorn
Yes
patent pivot
!int e print(bot.get_cog("ModLog"))
stable mountainBOT
In [30]: print(bot.get_cog("ModLog"))
<bot.cogs.moderation.modlog.ModLog object at 0x7f61e06a7d90>
got it
Maybe I'll add tests for the regex cog first, and then I'll move it to snekbox.
Running docker-compose every time doesn't sound like lots of fun
green oriole
You can reload the cogs on the fly
celest charm
true
green oriole
You don't need to reboot the container
mellow hare
Done that while testing
celest charm
Still, writing tests is a good idea.
mellow hare
Absolutely
green oriole
TDD can be fun sometimes
crude gyro
I don't love cogs referencing each other
I prefer moving any function that is useful to several cogs into a util file
and importing it into both
clever wraith
we should be able to disable or remove one cog without breaking any others
crude gyro
yes.
so imo @celest charm @patent pivot
we should not reference one cog from another. we only do this for modlog and even that is slightly dubious
better to just make more utils.
tough imp
I don't think cogs should ever import across each other, d.py will force-reload the module
so the definitions can run twice
I ran into a problem with seasonalbot where something was reliant on import order as a result
celest charm
So maybe I (or someone else) will move that in a separate PR, then I'll git pull, and then work with that
crude gyro
I don't think you need a second PR
patent pivot
yeah, as long as it's documented that things have changed it can go in this PR
crude gyro
it is well inside the scope of a new cog to utilify other cog's methods
green oriole
As long as you move the function in a separate commit, it should be fine yeah
brazen charm
I don't think it's worth the trouble in this case; unless a whole set of utils for snekbox communication is created
hardy gorge
I agree with lemon; I don't think the design pattern of referencing other cogs is particularly well
The modlog cog references are luckily fetched when needed (instead of being cached once), with cog reloading in mind
patent pivot
yep
hardy gorge
However, discord.py comes with a dispatcher/event listener utility built in
green oriole
I think that prideavatar size isn't working correctly
.prideavatar LGBT 64
Unh
Well, see the last two messages in #sir-lancebot-playground
No matter the size, it seems to always have the same size
molten bough
I sent a DM to @patent pivot a while back because my Font Awesome CDN sub was going to expire in a couple months, and you guys are using it
because I still need it myself, I've renewed it, so you can keep using it
they decided to give backers over 50% off to renew so it was worth it
patent pivot
oh sweet, cheers for that 😄
molten bough
it also includes font awesome 6, whenever that comes out
interestingly, font awesome 6 is going to have an official django plugin, so that'll solve some issues
patent pivot
huh
molten bough
you guys can read up on it here if you're interested https://fontawesome.com/6
green oriole
Yeah I don't know what's up with my pfp, maybe that's a cache issue
molten bough
Oh, also, there are various services-oriented features that FA6 will bring (like adding icons to kits and the desktop subsetter)
when those eventually release, if you guys want to use those, I'll give up to 3 of you access to the account
(I'd give more but I don't have enough seats)
celest charm
Maybe I'm weird, but I really doubt using fontawesome in my projects. It can make a website look dull and uninteresting because it has the icons I've already seen a thousand times.
Like, the basic login, logout, user, eye icons
molten bough
That's fair, some people aren't a fan
I find most of the utility in icons is that they're instantly recognisable though - you see that icon, you know what it's for
FA is just the most common - there are other sets out there, like the material icons site (which is mostly community maintained), and those are nice too
celest charm
I would love to learn drawing beatiful icons myself.
molten bough
Me too, but I'm nowhere near that inventive :>
tawdry vapor
This site has a lot of icons in many different styles. They're not fonts, and I haven't used them for web, but I do like them https://icons8.com/icons
molten bough
Yup, icons8 is real nice as well
oh, speaking of fonts, the PyDis kit is using the web fonts version - FA says the SVGs perform better and are more versatile, I recall just picking that version since that's what we used way back when
it's just a toggle on my end if you want to change
celest charm
This is the icon set I drew in Inkscape. I think it's mostly consistent, but some icons are really shitty. But it works for my purposes.
celest charm
👍
molten bough
although for real, having a custom icon set like that really makes you stand out
celest charm
Well, that's enough for my project
Another benefit is that it occupies 1.6KB when gzipped
molten bough
Yup, that's also nice
FA's svg kit still only loads what you use, but it's guaranteed to be far more than that
far, far more
celest charm
Although the 1.6KB bleaks in comparison with the giant JS libraries I'm loading
molten bough
You should webpack it
celest charm
I probably can live without jquery, though
But I don't think I should worry about performance right now.
I'm getting 90+ with lighthouse
The biggest increase in performance while browsing (as opposed to loading) was in switching to an SPA.
molten bough
That can definitely have an effect, yeah
Anyway, I think we might be getting a bit OT here, haha
molten bough
aight
dull jetty
THoughts on avoiding dormant-ing a help channel if someone's typing in it?
also thoughts on why my tshirt is so small suddenly
patent pivot
think the typing thing is proposed
dull jetty
oh nice
cold moon
But currently question is how to implement this, I think.
hardy gorge
With redis caching in place, it should be trivial to keep track of the last "typing" time in a channel
And check when the timer runs out when that was
the on_typing event gets the datetime the typing event started anyway
cold moon
Woah, this sounds nice to solve
hardy gorge
I think I proposed the same thing, without redis as that wasn't there yet, in the issue
dull jetty
there can be a considerable delay between typing starting and that message being visible to others
the typing notification sorry
i might try and determine what the typical and worst case timings are
if it's significant you might end up having to undo a channel dormant-ing
particularly given this feature would mostly come into play (from a user's perspective) when a channel is about to be
ok i don't want to have to keep conjugating 'dormantify'
'sleep' a channel
hardy gorge
I don't think we have to extend the scope that far just yet. If it's mere seconds between "member starts typing" and the dormant action, the message they were typing shouldn't be too long yet. At the very least, the member should be able to type it again in the same one or two seconds they had before the channel went dormant and their channel message box was cleared.
Because making a channel "undormant" would still mean that their message box was cleared anyway
So, I'm not too worried about those few times a user literally starts typing in the very last two seconds before a channel is made dormant
dull jetty
hmm yeah
there's also the argument that the point at which a channel becomes dormant might be the point at which there's no longer a need for help, if someone forgets to close
a surprising amount of people ask for help, then someone asks for clarification and they're never seen again
tawdry vapor
Noticed an issue with our site when hovering over "more" in the navbar to show the drop down. It causes the scroll bars to appear, which cause the entire page to shift to the left jarringly
brazen charm
Doesn't seem to be the case for me on chrome and 1920x1080 display
tawdry vapor
Which page did you try it on?
I tried it here
On chrome, it's fine. Not on Firefox
brazen charm
Seems to be fine everywhere I can quickly get to on chrome
tawdry vapor
Well the page must not have a scroll bar initially, and I was having trouble finding any public pages short enough
green oriole
Have you seen this? (I guess so, but in case) https://github.com/advisories/GHSA-2m34-jcjv-45xf
green oriole
Well, dependabot is opening PRs
dull jetty
hopefully all the pydis stuff is already latest though?
i assume they only made the cve public when the patched version was largely being used
clever wraith
ok so
but like, all i did was add a tag file, so im a bit confused
if anyone has any advice, that'd be welcome
@clever wraith looks like you have trailing whitespace on a line somewhere
ahh
oki
how did you see that?
go to the link you posted, click here
wait did i just not see that
cuz i was there :S
then select the failed job (lint & test) and the failed subjob
would a newline at the end of a file do it? cuz like there is no trailing whitespace S:
nvm i found it
I think it's the trailing whitespace in the code block
it was line 4
had a single space
ah
yay
tyyy
👍
tawdry vapor
That was simpler than I thought
Didn't realise we use their API to get all the info
In hindsight, that is obvious
brazen charm
What's the most normally used pydis project?
tawdry vapor
My guess is flake8-annotations
green oriole
Not bulma?
Indeed
532 downloads last month for simple-bulma, 12.9k for flake8-annotation according pypi stats
molten bough
I also use it, it's fantastic haha
molten bough
In setting up for another set of projects, I stumbled across this document
This would allow pydis to centralise these documents in a single repo, which might make organisation easier
patent pivot
interesting
molten bough
I'm also using probot to centralise some other stuff but that might be a bit much for pydis
labels, eg
patent pivot
i'm planning a bespoke github bot
patent pivot
stinky js 👎
molten bough
haha
dusky shoreBOT
tawdry vapor
.issue 910 bot
tawdry vapor
.issue 988 bot
dusky shoreBOT
brazen charm
Was wondering about this when I was reviewing something few days ago. For contributors is there any use in reviewing something simple like 935?
tawdry vapor
We need to discuss this actually
I wish your reviews would count
Of course they are still helpful regardless
Maybe we can just force merge something if it gets 2 approvals, regardless of whether they counted or not
patent pivot
I think it's still useful, means that the second core dev reviewer has less to look at and can get things merged
tawdry vapor
Having to wait for a 2nd core dev slows down the process. There's no reason if we already have a perfectly competent 2nd review.
Well, there may be a reason and that would be limitations of GH's permission system.
I dunno if it's possible to let all core devs force a merge, especially to a protected branch
brazen charm
with big PRs I don't worry about it making a difference, but in cases like 935 where it adds one simple block that at most has to be tested functionally in one or two minutes and doesn't really save any time for the core dev reviewer
crude gyro
I dunno if it's possible to let all core devs force a merge, especially to a protected branch
@tawdry vapor
a much simpler solution is just to set the minimum required approvals to 1 and adopt a policy to wait for 2, where the second can be from anyone
tawdry vapor
I'm OK with that compromise.
crude gyro
I've suggested it before, I don't think it's very controversial.
I'm in bed but I could probably fix it tomorrow
crude gyro
if it can wait a week we can bring it to the staff meeting.
but personally I don't anticipate any resistance
maybe a core dev ping would be enough
tawdry vapor
Yes. Not all of them can make the meetings anyway
Do a little poll in the channel
That can wait for tomorrow, you don't need to do it on your phone
crude gyro
oops
cold moon
https://github.com/python-discord-bot What is this?
tawdry vapor
A github bot
patent pivot
lol yeah
dull jetty
python discord github bot
as opposed to @stable mountain , which is a github python discord bot
and !e, which is a github discord python bot
it's already doing better than me
patent pivot
lol
green oriole
Joined 11 hours ago
There wasn't already a github bot?
patent pivot
@green oriole there is https://GitHub.com/pydis-bot but I didn't want to spam everyone's emails and change a load of auth stuff lol
cold moon
But this bot is not open source?
green oriole
I mean, the bot haven't done anything yet
cold moon
This have been done at least one thing: https://github.com/python-discord/site/pull/349 labels addign
adding
green oriole
Oh well, it doesn't show up in activity
cold moon
What lang this is?
mellow hare
Ruby?
patent pivot
Elixir
cold moon
Is this in Elixir because Python don't have library for this?
patent pivot
I just prefer the elixir libraries for github & wanted to do an elixir thing
cold moon
OK
patent pivot
plus this is clean
green oriole
The font for payload looks weird
patent pivot
cursive
green oriole
But yeah that's pretty clean
That reminds me of time when we wrote code on paper at school lol
But that could be done with a bit of flask too :>
cold moon
I heard somewhere that PyDis have some plans to make GitHub approval requirement 1 review and then 1 from someone who is not Core Dev (of course this may Core Dev too, this is just not required). When this idea will be applied, then maybe should bot check on every review, is all these requirements passed and when it is, auto merge PR?
tough imp
no I disagree strongly
brazen charm
Auto merging doesn't sound like a great idea
tough imp
merging to master should always be done by someone who is present
so that they can make sure everything went fine or revert if necessary
patent pivot
yeah we won't auto merge
tough imp
i've left PRs at 2 approvals before because I wasnt around my computer at the time
cold moon
Then maybe this should attach label like status: ready for merge
patent pivot
we could yeah
green oriole
You loose the merger name in the git history though
patent pivot
if we auto-merged?
green oriole
Yeah
'cause the bot would have merged it
I guess it could leave in the commit body the person that authorized the merge
woeful thorn
I don't see a point in a ready for merge label
patent pivot
yeah, or we could not auto merge
woeful thorn
GitHub's UI already shows this
patent pivot
@woeful thorn not if we are doing it for contrib reviews
we still need 2 reviews, but it will be marked as ready at 1, but we won't merge at 1
woeful thorn
It's still not necessarily ready for merge
patent pivot
from the ping in #dev-core, it should be
1 core dev review & 1 contrib review should mean merge ready
green oriole
But how do you count if someone is a contributor?
patent pivot
wdym?
green oriole
You'd have a separate list or a gh team?
brazen charm
Any person that contributes is a contributor
cold moon
Maybe DB with bot?
green oriole
Ah, you aren't talking about auto merge
clever wraith
I saw do up there I thought someone invented a cursed Python language, that's crossed with Python and Lua.
patent pivot
!tempmute @bitter vigil 12h Please don't link drop across multiple channels, that is spam
stable mountainBOT
:incoming_envelope: :ok_hand: applied mute to @bitter vigil until 2020-06-11 03:36 (11 hours and 59 minutes).
clever wraith
your github bot
needs to make up its mind
:o
unless thats 1 additional review?
crude gyro
@patent pivot
clever wraith
also if you update after a review, should you like re-request the review?
or just leave it
molten bough
That looks right, a commit would invalidate a review
patent pivot
@crude gyro intentional
a commit invalidates the review so it wants two reviews again
oh wait
clever wraith
so it is additional reviews?
patent pivot
but it didn't remove the one review
crude gyro
that's annoying and not how a human might treat a commit.
patent pivot
oh yeah it did
clever wraith
patent pivot
hmmm
clever wraith
also if you update after a review, should you like re-request the review?
@clever wraithimportant question, i dont wanna bug people if i dont have to
cuz like i literally commited the changes joe suggested
but
crude gyro
let's say I approve a PR, but mention a few minor nitpicks that the author can decide to change. or not. then the author makes those changes. Why shouldn't my approval count?
I think dismissing stale reviews should be a human decision
clever wraith
that sounds sensible
crude gyro
and another thing
your bot is switching the labels when someone leaves changes requested
why? that's not an approval.
clever wraith
i didnt leave them requested thats the thing
patent pivot
well yeah, it does and then when it gets to no reviews required it flicks to waiting for author
crude gyro
hm. right.
clever wraith
i literally commited them both, so i dont understand why github is saying changes requested =/
patent pivot
it's not 2 approvals required it's 2 reviews required
crude gyro
I think it should be waiting for author the minute there's a changes requested
crude gyro
it's not
2 approvals requiredit's2 reviews required
@patent pivot
hang on. why isn't it the former?
we shouldn't be merging stuff before we have two approvals, specifically.
I mean, that's how we communicate that something is ready to merge
patent pivot
yeah, for sure, but this isn't for mergability, it's so reviewers know where to look
we can do the former
clever wraith
off topic. but im semi confused. why does github still say joe wants changes ;-;
crude gyro
when you have one approval from a core dev and one from a contrib, isn't it supposed to put some sort of ready to merge label?
woeful thorn
Because he has to re review
crude gyro
how is github supposed to know that you made the changes correctly?
joe needs to approve the changes you made
clever wraith
well.. i commited his exact changes
patent pivot
ELA didn't like a ready to merge label, and convinced me as well
clever wraith
so i just figured github would consider those are 'correct'
also if you update after a review, should you like re-request the review?
@clever wraith hence this
patent pivot
yeah, you can re-request the review
crude gyro
okay
yeah I read ELAs statements now and I agree too
anyway I'm just stream of conciousnessing
woeful thorn
Spooky
crude gyro
sorry that it's a little nebulous
patent pivot
so flow looks like:
PR OPENED:
+ needs 2 reviews
+ status: needs review
1 REVIEW:
+ needs 1 review
+ status: waiting for author (assuming it's request changes)
+ status: needs review
2 REVIEWS:
- status: needs review
+ status: waiting for author (unless it's 2 approvals, in which case just remove all review labels)
okay. that seems reasonable to me
patent pivot
and we don't discount reviews if there were new commits
woeful thorn
@clever wraith GitHub has an option to dismiss reviews when commits are made. We’ve disabled it because it’s really annoying
clever wraith
ohhh
but like. the exact commit?
it's not the review itself, but the fact i made the exact changes joe asked for, so how could he still be requestiing them
thats my thought process anyway
woeful thorn
Any commit
crude gyro
dismissing reviews on commit is not very productive.
patent pivot
also apologies for label spamming your commit @clever wraith lol, just trying to crack this
crude gyro
it means the same people have to keep coming back to click a button, often for no reason at all
patent pivot
should we dismiss the status: waiting for author on new commits?
clever wraith
i mean that'd make sense
patent pivot
okay @clever wraith rereviewed, think that's my final comment on the matter
also I'm just going to use your PR as a guinea pig for something
okay it worked, nice
patent pivot
that was you being my guinea pig
I needed a new contrib to the repo
PRs will not be messy like this in future I promise
cold moon
https://github.com/python-discord/bot/pull/957#discussion_r438384953 How I should test does this exit enough early?
cold moon
I need help. Something went wrong when I updated branch from pydis master in https://github.com/python-discord/bot/pull/957 and now I have so much commits there.
hardy gorge
Try changing the target branch to something other than master and back again
That should make github recalculate the diff
cold moon
Thanks, this fixed that issue
tawdry vapor
I still see a bunch of commits
@cold moon You definitely duplicated your commits somehow. You need to fix this.
There are commits with the same message but different hashes.
cold moon
Huh, I don't understand what went wrong...
hardy gorge
Did you try to do a rebase?
cold moon
I tried merge, but then something went wrong and this failed. Then I readed from Stackoverflow that I have to do rebase, so I did
tawdry vapor
You need to rebase to delete those duplicate commits, and then force push
cold moon
Ok
celest charm
I'm sorry that I've been ignoring my re command PR, I'm busy with other stuff right now. If someone is interested and knows the snekbox interface, feel free to take over.
sullen phoenix
markdown isn't properly escaped: https://discordapp.com/channels/267624335836053506/267659945086812160/720850568234663986
if someone could open an issue for this, that'd be great. i'm a bit busy at the moment
maybe also fix Flask to Django: https://discordapp.com/channels/267624335836053506/267659945086812160/720862086921453618
cold moon
@tawdry vapor I have problem: When I use Sublime Merge for rebasing, I don't see duplicate commits.
tawdry vapor
Check out your branch again from the remote. Maybe even using git reset --hard
I don't use that program so I cannot help you with it
I see the duplicates in git log
cold moon
Yes, I see them with log command too
tawdry vapor
Do a rebase from the command line
cold moon
OK
clever wraith
@celest charm I might take over. Is it all detailed in the PR so far?
clever wraith
ack, will crack on with the todo
celest charm
Thanks!
clever wraith
the bot added a label twice
patent pivot
@clever wraith think I know why that bug happens, checking it
clever wraith
oki
https://github.com/python-discord/bot/pull/991/files#r438924530
not too sure what mark meant by having a scheme
brazen charm
http(s)...
patent pivot
it's trying to calculate what reviews are outstanding and somewhere it is slipping up
patent pivot
codeql yeah
clever wraith
o?
clever wraith
ohhhhh
patent pivot
i signed the org for the beta and we got access like... yesterday
clever wraith
oh fun
patent pivot
okay I think I've fixed the waiting for author bug
brazen charm
So contrib reviews are now working fine or is it still in the process? Got one or two pre where I left it at a comment instead of a putting an approval after they were addressed
clever wraith
i wonder if i should pr a cooldown (deco) tag
cuz like there are so many d.py tags that'd be useful
patent pivot
the issue was that it calculates the number of approvals and number of outstanding requested changes. approvals was < 2 so it added waiting for author but requested changes was 0 so it tried to remove it. approvals now does not influence waiting for author, only requested changes does
clever wraith
ohhh
patent pivot
@brazen charm contrib reviews should be good to go. to keep github UI all working like anticipated we've kind of said that we'll keep the requirement at 2 but if a contrib reviews and approves then we'll give it a once over and add a core dev approval and merge with that
brazen charm
Not really a fan of having s tag for every dpy feature out there
cold moon
Maybe in one day GitHub allow itself 1 write access approval and 1 contributor approval requirement.
patent pivot
potentially yeah
if github adds better permission granularity then we could look into having contribs in the org again
clever wraith
Not really a fan of having s tag for every dpy feature out there
@brazen charm
True but the basics? A basic command, how context works, things like that?
brazen charm
That'd be more fitting for the wiki imo
patent pivot
okay i updated your branch and things seem to have worked, it removed waiting for author since you have committed now
poggies
clever wraith
Hm what would make something good for a tag
patent pivot
I think if we keep things short it could go in the tag
the things for the wiki are like uhhh
we have some long ones let me find them
!starimports
stable mountainBOT
patent pivot
!mutabledefaultargs
stable mountainBOT
patent pivot
those ones for example
brazen charm
Can't say I like them
mellow hare
God those are huge...
patent pivot
yeah, they should be going on https://pythondiscord.com/pages/resources/guides
brazen charm
I don't think there's much purpose behind a tag that needs a conversation around it or a lot of followups, which would be pretty common with discordpy
patent pivot
(which we have internal issue for, migrating tags => wiki)
brazen charm
And people just sending tags as responses to questions should be considered
patent pivot
but we do want to have a big collection of tags on all sorts
ideal tag length is probably
mellow hare
3 or 4 lines
patent pivot
!global
stable mountainBOT
mellow hare
Max
patent pivot
hmm that's a little long
mellow hare
!f-strings
stable mountainBOT
mellow hare
Jesus no
patent pivot
!with
stable mountainBOT
patent pivot
that's getting there
brazen charm
Or gotcha is fine imo
patent pivot
!relative-path
stable mountainBOT
patent pivot
!orgotcha
stable mountainBOT
patent pivot
yeah
brazen charm
Looks a bit worse when wrapped
clever wraith
So if I can make a tag for those that's short?
(the things I suggested{
Or would that still be better for the wiki
cold moon
Discord should add collapsing code blocks
patent pivot
We can have a tag and a wiki page
And have the tag link to the wiki page for more info
patent pivot
So yeah, I'm okay with more tags for d.py tags
that's a wiki system, helpers+ can add articles and stuff
clever wraith
how would someone use that... seeing as context is explained in the docs just..not well apparently
and ohh yeaaaa
cold moon
I like GitLab's permission system more than GitHub's one. There is multiple levels.
cold moon
Let's combine GitLab + GitHub lol
clever wraith
xD
i love how the 2 requested reviewers didnt even
wait im guessing they're code owners?
mellow hare
We've got a group of core devs that get pulled from a list when a PR is made
So the ones who get requested may or may not be the ones who do the reviews
clever wraith
ohhh
mellow hare
More just trying to spread the work load around
clever wraith
also. what happened here
https://github.com/python-discord/bot/pull/991/commits/776f80fe53729d9c37aedd14784c81bc28c2e901
i at first thought it merged, but the pr is still open so-
brazen charm
Can't say I saw a lot of reviews from people on the prs where they were auto requested
mellow hare
It's currently blocked by another PR
clever wraith
ye?
mellow hare
No I mean from your link I couldn't see the file at all
brazen charm
master was merged into your branch so the pr can get marged into master
mellow hare
No I getcha now
clever wraith
oH
mellow hare
Oh right, so you can stay up to date
clever wraith
that makes sense
brazen charm
check any closed pr and that'll be the last commit if master got a commit while it was open
mellow hare
That or a rebase
clever wraith
so like why does that need to happen tho? i didnt touch another file that was edited
or is it just a thing done for all
mellow hare
You didn't, no, but other files in the main branch changed
And your copy needed to get those changes to catch up
clever wraith
why?
mellow hare
Consistency
clever wraith
hm
mellow hare
And to check if there's any conflicts
mellow hare
If you did something to one file and one file only, that isn't to say that changes from someone else's merges wouldn't cause an issue with your file
Yes, but it still does that by merging under the hood
clever wraith
hm ok
mellow hare
GitHub spoils us in a lot of ways
clever wraith
it also calls me out for my force pushes >:(
clever wraith
well it was either that or have a million commits, i was just amending one cuz i didnt realize i could use flake8 locally
mellow hare
Makes sense
clever wraith
cRaZyGmR101 FoRcE PuShEd FoUr TiMeS
so like, about the wiki, is there a way people can suggest to add to that or?
sullen phoenix
only helpers+ @clever wraith
clever wraith
i know helpers and up can add but is there no way to suggest?
sullen phoenix
i guess you could open an issue in the meta repo
clever wraith
oh, i didnt know that was a thing, alr
cold moon
Maybe GitHub bot should not add any labels when PR is draft?
tough imp
yeah that was a bug, we already adjusted it
cold moon
Can we get https://github.com/python-discord/bot/pull/817 merged soon? I'd like that when some changes is made that user itself update test not that I have to update branch and then update tests.
woeful thorn
It’s not like it’s sitting there on purpose
brazen charm
When were the github labels changed to the short versions?
brazen charm
It seems to be a bit harder to understand when focusing on the invdividual tags
Don't know what it was before but I can't get the i/L on the difficulty
tawdry vapor
It was level
That one is probably the most confusing
And I am debating removing it, since "good first issue" is adequate in most cases
brazen charm
I think apart from the priority the codes or how to call them are not really needed that much
tawdry vapor
You mean the numbers?
tawdry vapor
I do agree but it's a way to keep them grouped in the label selection list
Maybe that doesn't matter 🤷♂️
brazen charm
Ah, not familiar with the interface on the other side
tawdry vapor
In here
I usually use the search bar anyway so IDK how important it really is to keep them sorted
brazen charm
out of context 1 - high could be confusing but I think things like tests , feature or the aforementioned waiting for author are clear if it doesn't need them for other reasons
tawdry vapor
Right
brazen charm
Not sure if the letters would have much meaning for someone looking at the issues for the first time or after a while for example
tawdry vapor
They never were intended to add meaning really, just to keep them sorted. Most labels have a clear meaning on their own, which is why I felt shortening the prefixes wouldn't hurt the understanding of the labels but shorten their length and improve overall readability on an issue with many labels
cold moon
@tawdry vapor Jam cog tests review addressed
clever wraith
i need help
hardy gorge
Hello, @clever wraith, what do you need help with?
clever wraith
uhm
i am working on the antimalware for bot
but, i need to see the whitelist of files
hardy gorge
Are you working on our bot?
clever wraith
no
i am working on a new project
i just wanted to know, which files can be allowed
@hardy gorge any suggestion?
hardy gorge
Hmm, that's mainly up to you. We have a whitelist in our bot's constants (https://git.pydis.com/bot), which you could ook at, but what you want to allow is ultimately up to you. In any case, this channel is to discuss contributing to our projects; it's not a general help channel.
clever wraith
paginator buttons are impossible to see on amoled mode on mobile
maybe a white stroke is in order?
hardy gorge
Hmm, they are regular unicode emojis at the moment
Not custom emojis
Looks like Discord isn't handling that well in this color scheme
clever wraith
ah, fair
hardy gorge
I'm not married to these specific emojis, although this set has logical variants for "first", "previous", "next", and "last"
At the same time, we're running into reaction rate limits with our paginator, so mabye we should just simplify the interface anyway
I think the bulk of our public paginated embeds only have 2-3 pages anyway, so only having a "previous" and "next" button would probably be fine
And, as most are restricted to #bot-commands for regular members, maybe we can even do away with the deletion option as well
That would bring the number of reactions we'd have to add for each paginated embed down to two
woeful thorn
@mental estuary why are you spamming this video?
mental estuary
@mental estuary why are you spamming this video?
@woeful thorn this isn't a spam.
hardy gorge
Hey @velvet sail, we don't allow recruitment on our server. Also, this channel is meant for discussion about Python Discord projects.
velvet sail
patent pivot
@green oriole why is https://github.com/python-discord/bot/pull/892 stalled again?
You mentioned something about PATCH requests but I was unclear as to why
cold moon
@tawdry vapor About source command, can you now try again. Old solution worked well for me (MacOS), but now I made changes and these work too for me.
tawdry vapor
Yeah it works, thanks
green oriole
@patent pivot I think it was because we wanted to reschedule the failed message deletions, so we need to update the database entry iirc
patent pivot
Ah yeah opened an issue
brazen charm
is there a traceback for https://github.com/python-discord/bot/issues/908 ?
brazen charm
Thanks
cold moon
About https://github.com/python-discord/bot/issues/1011, maybe should this use run_until_complete instead create_task?
cold moon
I got such info that GitHub is already working on custom permission roles, but currently they don't know when this will be ready.
tawdry vapor
No that won't work since the loop is already running I think
patent pivot
the thing is with github permissions
even if they add more granularity it's going to be difficult to manage
one of the concerns we had was that a contributor could force push over another contributors branch and stuff along those lines, as well as managing issues and PRs which could also be not optimal (especially if it went through the GitHub API and such)
We'll see what they implement but personally I think write permissions for 3rd party members is a risk with little benefit (forks aren't that bad)
cold moon
I hope that this is like you can choose permissions one by one for every role and then when you assign role to user, and then user have permissions that this role provide.
patent pivot
From a contributor standpoint, what would the perks of being in the organisation be?
cold moon
So contributors should have like approve and require changes permissions.
brazen charm
Contributors can practically do that now
patent pivot
yeah, that's something we're hoping to implement through the github bot anyway
if we see a PR which has two approvals with at least 1 being from a core dev we will merge it
cold moon
OK, this is new for me
patent pivot
yep, we're just working some bot technicalities out (right now it still all runs on my laptop) and then I'll probably ping all contribs
cold moon
Not sure is this already implemented, but maybe should bot notify PR authors with long inactivity when changes requested.
patent pivot
We could look into that
We add the waiting for author label when changes are requested
brazen charm
Can just subscribe through GH and get it to your inbox
woeful thorn
I don't really like the idea of adding automated nagging
It's not really our responsibility to plan our contributors time for them
cold moon
But this should notify when this is like 3 months+ inactivity with waiting for author label.
sullen phoenix
so i can merge if there's 1 approval from a non core dev and the other from a core dev?
patent pivot
@sullen phoenix yes, a brief look over should be done but providing there are no outstanding requested changes that's fine
hm wonder why ks's requested changes didn't count there
I think GitHub api is returning some odd things
patent pivot
yeah
hah
nope
it was just my abysmal wifi speeds
I re-queued the labeller and it went through
patent pivot
lol
so yeah @cold moon, your requested changes there put the PR into a waiting for author state
tough imp
excellent, but what resolves the s: waiting for author tag then? is it when ks approves?
cold moon
I think when new commits come
tough imp
mm ok
patent pivot
However
sullen phoenix
is it possible to make sure all suggestions are resolved?
patent pivot
hmmm
tough imp
I think it'd make more sense on all suggestions resolved rather than new commits
sullen phoenix
yeah
tough imp
since I'll still do that but it's more conscious
new commit doesn't imply all is resolved
things may even be worse now
sullen phoenix
patent pivot
yeah
woeful thorn
Suggestion resolution isn't going to be deterministic enough, ignoring that GitHub's automatic detection is sometimes wrong, there can also be changes not tied to specific lines of code
Commits are probably the least worst reliable option
tough imp
there is no way to remove the tag add/remove event from the PR timeline right?
woeful thorn
No
tough imp
because if I push a commit and then manually set back to waiting for author on my own PR to communicate that I'm not done, it will still have the visual noise of fighting the bot
patent pivot
when looking through review comments the API does not return the resolved state
so I have no idea how that is calculated
cold moon
Maybe this should remove it when requester approve?
patent pivot
I'll look into altering the logic
neon phoenix
just lint before you push :)
cold moon
https://github.com/python-discord/bot/issues/1005 How to recreate this situation for testing?
cold moon
During solving https://github.com/python-discord/bot/issues/1011, I found one bigger issue: Bot.http_session is already closed before L99 at bot.py, because dpy's own Bot.close closes their internal session, but these sessions share same connectors, what means that both sessions will be closed. I tried waiting for access token revoke task finish, before L99, but session is already closed there. And I can't put this before await super().close() because there this unload cogs what mean task is not available before it.
cold moon
I have problem: When I use docker-compose up then web is missing some migrations, like avatar_hash removal.
tough imp
@cold moon Ah sorry about that (the resolved conversation), that's annoying
mmm I'm not particularly excited about contribs not being able to do that
cold moon
Contributors should at least have ability to mark resolved their own reviews, but GH permissions system is not best.
tough imp
yeah definitely
I marked the constants one as resolved as well then, although joe commented there too
cold moon
👍
But can anyone help me with this missing migrations when using Docker? https://canary.discordapp.com/channels/267624335836053506/635950537262759947/723818471762296985
patent pivot
I'll look into a better way to mark reviews as resolved @cold moon
Maybe adding a thumbs up reaction could do it
Providing you either authored the comment or are the owner of the PR
brazen charm
You can do it normally if you're the owner
patent pivot
Ah
cold moon
Anyone have ideas how to solve https://canary.discordapp.com/channels/267624335836053506/635950537262759947/723804876123144294 ? Bot.close (dpy) automatically closing Bot.http_session don't let wait until revoke task is complete (session is already closed)
tawdry vapor
I dunno, you need to find a way to block in cog_unload until the task finishes
The http session isn't closed until all cog_unloads have been called
tawdry vapor
cogs are unloaded in commands.Bot, and session is closed in discord.Client
I dunno, that may be useless since the bot awaits super
cold moon
Problem is that await super().close() closes self.http_session
tawdry vapor
Have cog_unload put the task in some collection and then do an asyncio.gather in close before awaiting super.close
That's a little clumsy but I guess it would work
cold moon
await super().close() is unloading cogs.
tawdry vapor
Righttttttttt
The task could just be added to the list in __init__
But I don't know if that may cause "unawaited coroutine" warnings
Alternatively, the close code from commands.Bot could be copy-pasted and then you'd have control over order of execution
cold moon
But then to inherit Client's close, is this super().super().close()?
cold moon
Oh, okay
@tawdry vapor Unresolved attribute reference 'close' for class 'object' I get this with await super(discord.Client, self).close().
brazen charm
I believe you want the Bot there if bot inherits from Client
cold moon
Thanks, this worked.
tawdry vapor
So how are you doing it?
Are you putting it in __init__?
I don't know what a good solution is
cold moon
Basically I created new Bot variable self.closing_tasks: List[asyncio.Task] = [] what this is awaiting on close. I will push changes and PR soon, more details there.
tawdry vapor
But the task should still happen in cog_unload too
That's the problem
Normally, create_task is fine. But it's not when the bot is shutting down
We want the same thing to happen when the bot closes as when the cog is manually unloaded
cold moon
This is creating it in cog_unload and appending to list.
tawdry vapor
I don't know how you're doing it then
brazen charm
I feel like the implementation should be properly discussed in the issue
cold moon
PR created. I'm going to sleep now.
tawdry vapor
I'd appreciate some discussion on this https://github.com/python-discord/bot/issues/800#issuecomment-647047778
brazen charm
Not sure what the actual problem is and if it's just with my setup but how does the mixed line endings precommit work?
It always fails in some cases, like currently when I'm trying to do a partial commit from pycharm, but when I view the line endings they are all CRLF. Running pipenv lint changes the endings everywhere and creates a diff
tawdry vapor
The hook doesn't do anything fancy
It just goes through every line in the file and gets the frequency of each ending
I think what's happening is that it forces LF endings
brazen charm
changing pycharm to use lf doesn't seem to be having any effect, but it does switch back to CRLF after it fails so I don't really know what's going on there
99% of the time I let it fail once to do its whatever and then it runs fine but this partial commit seems to break it in some way (pycharm switching out files?)
tawdry vapor
Try setting core.eol to lf and core.autocrlf to input
Or maybe set autocrlf to false 🤷♂️
brazen charm
changing it to input along with the eol seems to have worked (at least for the commit I was having trouble with), thanks
brazen charm
I believe I asked this before but is there any stance on naming/structuring the git(hub) stuff?
With commits I've been trying to be more descriptive because I've found myself wondering about what's something for and the blame not helping at all but it sometimes feels like I'm leaning into the implementation there, is that alright to go a bit into?
Then I've seen some coredevs structure their branch names according to the issue/feature they've been working on; guess this is more curiousity since it doesn't show up almost anywhere from a fork, but is that just a preference or does it go from some form of a guideline?
patent pivot
it's just preference really
on the master repo it's nice to try keep things tidy so we each have a way of claiming a branch as ours or relating to an issue
we do prefer more descriptive commits though, we don't squash and maintaining a nice blame is useful
we had the ban messages fix recently where ban messages were being sent after the user was banned and it was hard to track that down and there was no commit to revert so there's an example of why we prefer smaller but more commits
patent pivot
@brazen charm trying to understand the review dismissal stuff, is this the kind of review where you don't see resolve? https://cdn.seph.club/discord/😪😌😀🙉.png
brazen charm
patent pivot
Hm, that is irritating
I guess since the PR author can resolve we can leave it to them or core devs https://cdn.seph.club/discord/😹🙈🙇😄.png
but that is irritating
neon phoenix
lmao i just clicked accidentally the pencil icon of a file from python-discord/bot and it got forked to my account
sheeeeeeeeeeeeeeeeet
patent pivot
lol
tawdry vapor
Even worse, that will happen if you just press "e" on your keyboard
neon phoenix
github must fix that somehow
if you click edit to a file and you didn't actually edit it don't fork it
meanwhile in bot/bot/cogs/filtering.py
patent pivot
lol
neon phoenix
gdudes-pony-farm
patent pivot
lemon got angry at around fuckery I think
someone changed it to that from aroundfuckery
neon phoenix
okay so in cogs/filtering.py i found this line of code py from bot.constants import ( Channels, Colours, Filter, Icons, URLs )
but i don't know where is bot.constants located
brazen charm
bot is the top level package, so it'll be in the dir next to main
green oriole
someone changed it to that from
aroundfuckery
We did have a really important talk about this haha
As the boss said #ot0-psvm’s-eternal-disapproval message
tough imp
hey @cold moon, available for a chat?
i'm wondering why the test for send_private_embed fails for you when you remove the if statement
I don't think it should and I cannot replicate it
for all 4 test cases, the await args to user.send look like this:
Calls: call(embed=<discord.embeds.Embed object at 0x7f74f9bef5e0>)
Calls: call(embed=<discord.embeds.Embed object at 0x7f74f9bef5e0>)
Calls: call(embed=<discord.embeds.Embed object at 0x7f74f9bef5e0>)
Calls: call(embed=<discord.embeds.Embed object at 0x7f74f9bef5e0>)
I can also step through the tested function & confirm that the return gets awaited in every case
there's also await_count which is exactly 1 for each case
cold moon
Interesting that my result is different. Also test_first_fail_second_success_user_post_infraction is failing for me locally too, but success in CI. Maybe problem is in my computer.
And await_count is 0 for all cases what raise error.
tough imp
that's very strange
not sure what to make of it
if tests fail locally but not in CI there's something odd happening and you need to figure out what that is
cold moon
@tough imp I tried in my Linux computer too, but there this fail too. I don't understand what is wrong.
brazen charm
What's the pr number?
cold moon
817
test_first_fail_second_success_user_post_infraction is failing with:
AssertionError: Awaits not found.
Expected: [call('bot/infractions', json={'actor': <MagicMock name='mock.message.author.id' id='4466251904'>, 'hidden': False, 'reason': 'Test reason', 'type': 'mute', 'user': 1234, 'active': True}),
call('bot/infractions', json={'actor': <MagicMock name='mock.message.author.id' id='4466251904'>, 'hidden': False, 'reason': 'Test reason', 'type': 'mute', 'user': 1234, 'active': True})]
Actual: [call('bot/infractions', json={'actor': <MagicMock name='mock.message.author.id' id='4466251904'>, 'hidden': False, 'reason': 'Test reason', 'type': 'mute', 'user': 1234, 'active': True})]
so again missing await on exception I think.
brazen charm
Didn't look through the module but the tests pass when I remove the discussed condition. If you don't have anything staged try resetting to the remote
cold moon
@brazen charm I reset to remote, but test_first_fail_second_success_user_post_infraction fail however.
Looks like that this don't count await that have raised exception as call.
cold moon
I found this:
import asyncio
from unittest.mock import AsyncMock
my_mock = AsyncMock(side_effect=ValueError('test'))
try:
asyncio.run(my_mock())
except ValueError:
pass
print(my_mock.await_count)
print out 0, but:
import asyncio
from unittest.mock import AsyncMock
my_mock = AsyncMock()
try:
asyncio.run(my_mock())
except ValueError:
pass
print(my_mock.await_count)
print 1. This don't affect MagicMock. Tested using eval command.
cold moon
This looks like Python bug.
green mesa
hello guys
i wnt to ask one thing i was just reading py bot code for mute command
there no db is used
so incase bot goes off
how u manage to get time
to unmute
plz correct me if i am wrong
green oriole
We do store it into the database
The function right here does that https://github.com/python-discord/bot/blob/master/bot/cogs/moderation/utils.py#L60
cold moon
@tough imp Problem resolved. My both computers was using Python 3.8.0, but this bug fix was released in 3.8.1, this was reason why these tests failed for me.
green mesa
@green oriole for same it does with help channel also?
green oriole
I don't know about help channels though
I think they are just backed by a redis dict, or discovered at startup
patent pivot
Claimants are backed by a Redis dict, the times are figured out on startup though
green mesa
ty
sorry if it feels like non sense question but y u all prefer redis dict instead of storing in db
exotic ember
someone will correct me if I'm wrong but we use Redis as it lets us store in memory as a cache as well as store in physical storage as needed
green mesa
oooo means it store same thing at two places
tough imp
I think its mainly for convenience, storing key:value pairs of simple types that get updated/popped/read frequently has less overhead this way than making site api calls everytime, plus creating a new mapping in redis takes no effort in contrast with setting up a new table/schema in postgres
basically its almost as easy as making a new dict
so development can move a lot faster as well and stays in one repo
green mesa
makes sense ty for your explanation
crude gyro
@exotic ember @green mesa
correct me if I'm wrong
Well I don't think you're wrong, but it's a little misleading to say that we're storing it in two places. The RedisCache is just a direct interface for a database, there's no in-memory caching. Redis is a so-called schemaless database (sort of like a python dictionary), which means that we can store whatever we want in it without setting up a table first.
Basically the bot needs to be able to store two different kinds of data.
-
Complex data that is important to retain, such as infractions. Because it's important that we keep this data forever, we need to store it in a safe place and make regular backups. This is what we use the relational database for (we use postgres). This data can also have relationships to other objects, for instance an infraction is related to user who got the infraction.
-
Ephemeral and simple data, such as who claimed a certain help channel. This is data that will only be relevant for a short time, and that we don't care about making backups of. If we lose this data, it has no serious consequences. For this, we use the schemaless Redis database.
To manage the second type of data in Redis, we created this convenient interface called RedisCache. Every time you get data via a RedisCache, you're making a Redis lookup, so nothing is actually stored in the bot memory for very long.
patent pivot
We will probably have future usage for redis outside of Redis cache
redis supports lists and stuff which would come in handy
green mesa
hmm
split nexus
Hi. I have a question about the configuration of the seasonalbot in PyCharm (version 2020.1.2).
I have already done all the steps explained in the Seasonalbot's Wiki.
Now PyCharm is still telling me that "Package requirement 'discord-py==1.3.3' is not satisfied.
I checked the module version in Pipenv settings and it tells me that discord.py = 1.3.3.
When I edit the Pipfile and change "discord-py" to "discord.py" then the message in PyCharm is gone.
Also PyCharm complains that the module "aiohttp" is not listed in the requirements.
When I put that module also in the Pipfile, then this message is also gone.
I have two questions about that:
Do I have to edit the Pipfile in order to not get those messages, or can I just ignore them?
Is there a difference between "discord-py" and "discord.py"?
brazen charm
I'm not sure about aiohttp but the discord one is a bug
split nexus
Ah, okay. A bug where? PyCharm?
tough imp
yeah I think so
if you run pipenv --venv in the project directory it will show you where the venv is located in your system
instead of using the pipenv integration in pycharm you can select that you want to use an existing virtualenv and select the python at the path shown by the command above
split nexus
Okay, I tried a few things.
Appart from the message in PyCharm showing up everything seems to work fine.
I guess I'll just ignore the message for now.
Thank you
tough imp
the approach above will make it read the venv correctly but yeah if it doesnt bother you then it doesnt affect the code in any way
you may lose out on some static analysis or autocomplete, not sure
green mesa
Where is url passed or name?
How bot knows that d.py is the url which has to be scraped🤔
Or any other package
brazen charm
The coentents of the inventories of the packages which are stored in the api are all added to the inventories dict
brazen charm
For any package you want to check it needs to have an inventory, don't exactly remember the urls those are behind
green mesa
No coc.py is the module which I want to scrape
So how can I make inventory for any package
patent pivot
I don't know what you mean by scrape