@celest charm why would it be removed?
#dev-contrib
celest charm
Wouldn't it be duplicated later, when you merge your branch?
crude gyro
no, because a commit that contains changes that already exist is not a commit
celest charm
Good then
crude gyro
a pull request where all the changes have already been merged cannot be merged, because it contains no commits
basically the changes themselves are the thing
it's a bit weird, but it's a good system
mellow hare
Certainly eliminates redundancy
crude gyro
very resistant to duplicates
so when you cherry pick from me and your thing gets merged to master, and I want to merge into master, it will just not list that commit in my pull request
and that's great! I didn't want it anyway
it was messy and had many changes to f-strings
celest charm
Well, flake8 doesn't have anything to do with redis, so
Maybe it should be in its own PR?
Because it doesn't have anything to do with regular expressions either.
tough imp
i'm thinking that the pin is too lax if we get issues like this
crude gyro
yeah I agree
hardy gorge
There's no guarantee that with a stricter but still flexible pin, like "~= 3.7.9", these things don't happen.
Simply because not every maintainer cares that much about versioning
But, it will probably help
tough imp
do flake8 versions follow Python versions or is that coincidental?
hardy gorge
Not that I know of
hardy gorge
I'm not sure, it did not do that in the past
e.g. there was Py3 support before 3.0.0 and 3.0.0 dropped support Python 3.2 and 3.3
3.3.0 added support for Python 3.6
So, it does not look like they have a history of "minor version of flake8" follows "minor version of Python"
mellow hare
There would have certainly needed to be updates for 3.6 and 3.8, though
Since both introduced pretty significant syntax changes with f-strings and the walrus
Coo coo cajoo
toxic cargo
just saying hi
and I am running some into some issues on running the bot on windows, and wanted to see if anyone else is having those issues, so we can patch them
brazen charm
What kind of issues?
green oriole
If you have a home build, you should take a look at the pins 
cold moon
About Redis, we should use https://github.com/aio-libs/aioredis for async support. I think this is not hard to implement this to current base.
patent pivot
@cold moon it is
we had a discussion about this in the last staff meeting
think about how stuff is currently implementing, you can implement dict behaviour but there is no such thing as an asynchronous dictionary
await self.cache["key"]
is ugly and
await self.cache["abc"] = "abcdef"
is impossible
so as Ves says a rewrite is required, likely one that switches things out for methods like .get(key), .set(key, val) and basically wraps all the builtin redis defaults with automatic namespace generation
hardy gorge
There's also the question of which async-redis package to use
There are a few different ones available
It also means that we'll probably be better off implementing iteration with a different tactic (as opposed to what we're doing now)
One fetch of the entire name space and then return an iterator over that
Unless we start working with massive caches and want to be able to fetch them one item at a time, but I kinda doubt that
patent pivot
Yeah, we won't be able to use mutable mapping anymore so it'll basically be wrapping whatever redis library we use and the redis commands for hashes (which implements the most important ones already like keys, values, items, membership, etc.)
We'll also probably drop the JSON stuff
Since using different keys is safer and won't cause race conditions
if we used JSON we'd have to use an async lock for every key
hardy gorge
Why?
Isn't the race condition between one fetch and another fetch, not the non-async JSON parsing we do in between?
We could still have fetch_a, fetch_b, post_a, post_b race conditions with a single value
just ask the GIL
Or do you mean if we fetch and post back whole dicts?
It's a bit of the same problem, but we're less likely to get it with single values with the things we're currently doing
but things like counters (for custom ratelimits and what have you) could very well suffer from the same race condition
cold moon
I think aioredis is best choice, because this have most active development when I googled
hardy gorge
It's one of the packages we've discussed, but aredis seems to have more active development
aioredis officially doesn't support Python 3.8, but I'm not too worried about that
patent pivot
right we could still have single value race conditions, but it's much more likely to happen with json stuff
hardy gorge
It just seems like they never got around to testing it for 3.8
@patent pivot Yes, you're making more values independent from each other
brazen charm
What data is planned to be stored in redis? Just out of curiosity
patent pivot
- help channel claimants
- unanswered help channels
- maybe cooldowns
- maybe posted news items
hardy gorge
Things like channel->claimant and "answered"-channel caches
Probably more things that need caching, like non-default settings values for our bot
(DEFCON comes to mind)
patent pivot
yeah
cold moon
https://github.com/python-discord/bot/issues/717 this is too something for Redis (amount of OT name uses)
patent pivot
hmmmm
isn't there a site PR for that though
and it does seem like something the site would like to know about
we can't just reroll until we find a combo of 3 which aren't used
patent pivot
I'll have a look now
cold moon
@tawdry vapor Addressed reviews in @stable mountain repo PRs 950, 945, 956
cold moon
About Redis, when PR get merged maybe we should get docs page about using this?
crude gyro
probably.
hardy gorge
Hey, @cold moon, are you here?
cold moon
Yes @hardy gorge
hardy gorge
I've been reviewing your PR for the tag tests, but there's a bit of an issue with the approach you've taken. Almost all of your tests rely on the production data (the actual tag files we have) to do their tests, even those looking that search tag content for terms or test the fuzzy matching method. This means that if changes are made to these tags (maybe one is deleted) or new tags are added that come up for these search terms, the tests will start failing.
One general principle in testing is to never rely on actual live, production data to do the testing. Instead, you create test data that you control.
That isolates the methods we're trying to test from the accidental/temporary state of production data
The way it could be done here is to create a "test data" folder with a few test tags in it.
That way, we can ensure that we know what all the functions should return, regardless of the actual tags we have or don't have in production.
cold moon
No, at line 11 I have CACHE variable what overwrite loaded tags
hardy gorge
hmm alright
I missed t hat
It still present for the first test method
but less severe
cold moon
Yes, this use this to test loading
hardy gorge
Yes, but it depends on the production data and it doesn't actually test that we're getting the right names/content
I still think that a better way of doing this is to ensure that the tests do not parse production data, but rather work from a test data directory
Because each time your setUp method runs, it will parse our production data
Alright, I got in the wrong mindset after reading the first test method, which does still depend on production data, and missed the monkeypatch cache you did
cold moon
Hmm, maybe I should patch this so this don't load production data? And in it's own tests, I should patch classes/functions inside it?
hardy gorge
It would be good enough if you could somehow point the cache loader to test data
You can do that by patching the Path-call in here to return a Path-object pointing to a test data dir: tag_files = Path("bot", "resources", "tags").iterdir()
That way, you could also validate that we actually create the right titles for tags and extract the right contents
Something that you now can't do because the production data is dynamic
(You only validate that the keys are present in the cache)
cold moon
Okay, I try to fix this
cold moon
@hardy gorge About that prefix issue, maybe I should import FOOTER_TEXT from tags file. Then when anyone change this, then test don't start failing instantly.
hardy gorge
That could be a solution, yeah
I didn't dive too deeply into how to solve it
It's just that having a few custom, but valid settings in config.yml shouldn't result in tests breaking
On our shared dev server, most of the core devs use a bot with a personalized prefix to avoid 10 bots responding at once
cold moon
How to patch class so during instance creation this return specified instance?
hardy gorge
You can patch the class object itself
!e
from pathlib import Path
from unittest.mock import patch
with patch("__main__.Path") as m:
m.return_value = "hello"
print(Path())
stable mountainBOT
@hardy gorge :white_check_mark: Your eval job has completed with return code 0.
hello
Thanks. I pushed changes @hardy gorge
faint vessel
print("hello")
from pathlib import Path
from unittest.mock import patch
with patch("__main__.Path") as m:
m.return_value = "hello"
print(Path())
!e
stable mountainBOT
You are not allowed to use that command here. Please use the #bot-commands channel instead.
lament totem
Hi , (unable to proceed ) where/how do i enable the developer mode & go to the User Settings ?
sullen phoenix
it's the gear down at the bottom left
the rest of the instructions should be clear enough i think
@lament totem
lament totem
@sullen phoenix thanks !
lament totem
Hi , Trying to run "pipenv --three sync"
Gives me the following error, how can i tell VSC to use my environment "discord-dev" which contains Python 3.8 ?
crude gyro
as long as 3.8 is available, it should create it for you when you make your pipenv initially
maybe the --three flag made it default to your system py3 instead of the one required by our pipfile?
try pipenv --rm to remove and then just pipenv install
essentially a pipenv reinstall
lament totem
try
pipenv --rmto remove and then justpipenv install
@crude gyro there is no output on console , is that normal ?
crude gyro
not really, no
in my experience, using the in-editor consoles for this sort of thing is a bit hit and miss
maybe just do it in a normal terminal
lament totem
i ran pre-commit run --all-files.
this error is present before i even stage any change. is something wrong ?
crude gyro
most likely your editor is not respecting the line endings we want
and has changed them to windows-style line endings
lament totem
is there a way to fix them real quick ?
tough imp
when you do git status, have those files changed?
lament totem
nope
tough imp
the linter runs on local files, not the latest commit
lament totem
so what is the fix for my issue ?
i am fairly new with dev-contrib, all the hooks , linter & flake8
@tough imp i guess you are right ! i just found all the files were modified
tough imp
"show_line_endings": true
include this in your sublime settings profile
you should then see the chosen line endings in bottom right
if you click it (for me it says Unix), you can choose which to use
it's probably saving files with windows endings (crlf)
but our projects use the unix ones (lf)
lament totem
@tough imp Thanks ! no issues now
tough imp
perfect
lament totem
where can i find the value for env variable --->> SEASONALBOT_ADMIN_ROLE_ID ?
CHANNEL_ANNOUNCEMENTS as well
brazen charm
You need to create the roles and channels in your server
lament totem
i already have
brazen charm
then with the dev tools enabled you right click on them and select copy id
lament totem
role and channels both
brazen charm
lament totem
@brazen charm Thanks able to copy the ID's
getting the following error when i run - pipenv run start
tough imp
yeah we have a PR open to fix that but it's not merged yet
you want to go on that line and specify encoding="utf-8" in the open call
it fails because on windows the encoding defaults to something else and that one resource file contains something weird that won't open
wait, it doesnt show the line where you need to specify the encoding
1 sec
it will be this line
change it to p.open(encoding="utf-8") locally and it should open
PR is here, we should probably work to get this merged soon if any core dev is around: https://github.com/python-discord/seasonalbot/pull/413
looks like it already has 2 approvals but I cannot merge it
maybe later today
lament totem
change it to
p.open(encoding="utf-8")locally and it should open
@tough imp works !! my bot is connected.
So one question , what can i do or what i can i use it for ?
patent pivot
You can see all the issues at https://GitHub.com/python-discord/seasonalbot/issues
lament totem
@patent pivot what i meant was...
Lets say we have build the application "calculator":
we we can do is :
Add
Subtract
Multiply
issues(features) would be :
divide
can't add Floats
etc etc
So i meant to ask was... what can i do with seasonalbot ?
dusky shoreBOT
Command Help
patent pivot
just run the .help command in #sir-lancebot-playground and you'll see
lament totem
in my server you mean ?
patent pivot
anywhere that you have an instance of seasonalbot
seasonalbot is pretty heavily tied into this server, so not all features will work on other servers
(or at least on a server not setup like this one)
lament totem
is there a list of things i can try with seasonalbot ?
where can i find it ?
patent pivot
yes, the .help command
run it in #sir-lancebot-playground or in your own servers #sir-lancebot-playground and you will see a list of all features
lament totem
Amazing! @patent pivot can see the list of things i can try !!
one more thing ! so i see the python-news channel... is that a seasonalbot too ?
patent pivot
that is not seasonalbot no
that is @stable mountain
you can subscribe that to your server but that has a more complex setup
lament totem
oh ! that will be such a drag... but worth trying the complexity and get to learn new things i suppose
patent pivot
you'd need to run a copy of our site as well locally
which wouldn't be too hard with the docker compose I don't think
lament totem
docker is out of my scope ! don't have a professional version of Windows
the site is amazing though !
how does the help bot work ?
lament totem
most of the amazing stuff is in @stable mountain ?
patent pivot
seasonalbot contains our fun commands which don't require lots of infrastructure, python handles the heavy lifting
help channels, moderation, antispam, off topic names, reddit, reminders, snekbox, stats and verification are all handled by Python
lament totem
@patent pivot Thanks for all the briefings ! 😄 guess i will look into seasonalbot functionality first then move to @stable mountain
patent pivot
Sure
Are you planning on contributing or running it on your own server?
Python is very locked into a Python Discord-like server, so it'll be harder to run that
lament totem
@patent pivot planning to contributing ! was speaking to Leon, he kind of got me motivated into the contribs
patent pivot
Ah for sure for sure, then yeah, the issues pages on any of our repositories details the specifics of what needs addressing, if you need any help or clarification feel free to drop a message on the GitHub issue or here and a core developer will get you an answer
cold moon
I rebased my clone of my bot repo fork and this have now some commits from python-discord/bot repo master branch and this want to push them. Will these show up in PR?
lament totem
@patent pivot Haven't been into any of the github contributions... this would be my first ! hence you will see lot of questions from me 😋
tough imp
@cold moon the PR on github shouldn't show commits that are already in the target branch
if I understood you correctly
if you rebased though you will probably have to force push
since the histories have diverged
so I dont know actually
cold moon
I try to force-push. Let's see. Next time I use merge. Much simpler
tough imp
I'd advise you against force pushing if you dont know what the outcome will be
cold moon
Oh, I did normal push and when I compare, I see only these commits that I made.
tough imp
if you want to merge upstream master you can always throw away your local branch and start afresh with what's in the remote
alright, sounds like it worked then
lament totem
other than .help & everything in the 11 page bot commands , what else can i try ?
tough imp
ok I have to look at the encodings PR
sullen phoenix
i was having a quick look earlier but didn't submit anything. there's a few opens with the encoding missing and the example in the doc string of bot.utils.persist.make_persistent should be updated to include the encoding as well
sullen phoenix
awesome 😄
lament totem
Hi , can i get started with issue #398 or are there steps to follow before i take it up ?
sullen phoenix
.issue 398
sullen phoenix
it looks like the issue hasn't been approved yet
patent pivot
Looks like @cold moon was also interested in taking that one
sullen phoenix
if you can't find any issues that you're interested in, feel free to open a new one with an idea
brazen charm
https://github.com/python-discord/seasonalbot/issues/377 sounds nice, it has the wip label but don't see any assignments
lament totem
sure ! @sullen phoenix
let me have a look @brazen charm
lament totem
So for the issue 337 - the output expected is title & link(as in image or the link to the image) ?
crude gyro
the output should be the image itself - or a direct URL that will embed in a way that shows the full image
the best user experience is to read the comic right here on Discord
not to have to click a link
green oriole
docker is out of my scope ! don't have a professional version of Windows
@lament totem I made a small tutorial about how to run the site on windows home, it has been integrated inside the setup guide, but I’d recommend you to read it there https://gist.github.com/Akarys42/989dcfa41536c9f3b15c844ea4f04d72
tawdry vapor
I need help with testing redis
I want to test the PR with the real deal redis, not fakeredis
The service in docker-compose isn't publishing the port so I'm starting it like this instead docker-compose run -d -p 6379:6379 redis
patent pivot
hm
tawdry vapor
Now I have an auth issue
2020-05-24 16:22:39 | asyncio | ERROR | Task exception was never retrieved
future: <Task finished name='Task-24' coro=<Bot._create_redis_session() done, defined at /home/mark/repos/python/bot-pydis/bot/bot.py:51> exception=AuthError('ERR Client sent AUTH, but no password is set')>
oh right
tawdry vapor
I don't know what to put for a password
patent pivot
that's a bug then
we should allow for a password not to be set and not send auth if it isn't present
tawdry vapor
That's just a code thing regarding how the pool is created then?
patent pivot
yep
aioredis expects a password in the REDIS_PASSWORD env var and won't tolerate not having one I guess
and redis is erroring because it's sending something or other
though I wonder what it is sending
tawdry vapor
I think i have it set to an empty string, the env var
tawdry vapor
I can try, but I think the config tests will then fail which is annoying
patent pivot
ah right yeah
tawdry vapor
But you're right, unsetting the env var does work
Which types does redis support as keys?
Is it the same types it supports for values?
patent pivot
just strings
tawdry vapor
Alright
patent pivot
everything is a string in redis
tawdry vapor
Oh yeah
Maybe our thing supports diff types for keys?
I wonder cause the trace logs are showing typestring prefixes even for keys
patent pivot
Right, yeah, maybe lemon did add that
tawdry vapor
Wow these trace logs he added are great
All the logging I wrote for my test is made redundant
patent pivot
async def get(self, key: RedisType, default: Optional[RedisType] = None) -> RedisType:
"""Get an item from the Redis cache."""
await self._validate_cache()
key = self._to_typestring(key)
thanks, I think that was ks's suggestion
but uh
maybe you can add exposing the port to the docker compose?
it's reaaally easy.
crude gyro
I described the process to Ves a little higher up in this chat
so there's a guide up there.
tawdry vapor
It's marked as required in the config and the config tests will complain if it isn't set
Oh it isn't required in config
Well, the tests complain anyway
crude gyro
the tests are a bit picky like that. they complain because None isn't a string
and because Optional isn't allowed in dataclasses
which is annoying
but we should probably make empty string work
I think empty string works for fakeredis but for real redis it might need None
tawdry vapor
We can add like password=password if password else None
crude gyro
so worst case scenario, maybe you can coerce empty string password to..
yeah that.
and that might solve it.
tawdry vapor
But the real fix would be to make the config and/or its tests stop being annoying
tawdry vapor
Ok well I will just make this fix for now cause I don't feel like fucking with config code
crude gyro
but..
mine is real redis setup
and I use empty string in config
and it works
so I am a bit confused
crude gyro
oh maybe you did empty string env var?
I did empty string in config.yml
@tawdry vapor can you try the latter?
tawdry vapor
Yeah, i had the env var set to an empty string in the dotenv file
I will try your suggestion
crude gyro
yeah, I think that works. not really sure why though.
tawdry vapor
No, that does not work
crude gyro
the fuck
tawdry vapor
Same error ERR Client sent AUTH, but no password is set'
patent pivot
tawdry vapor
redis:
host: "localhost"
port: 6379
password: ""
use_fakeredis: false
okay. it does work for me but I am running the bot through docker compose
tawdry vapor
Yeah, that is a difference though I do not see why it would affect this
crude gyro
that shouldnt matter, yeah.
tawdry vapor
You didn't mess with the redis container and configure a pw for it?
Well, we can also pass it a custom config to set a password https://hub.docker.com/_/redis/
I'd rather avoid that though
I don't think we even have persistence enabled for redis in docker compose so there's no benefit to using it over fakeredis
crude gyro
yeah, I just wanted a real redis for writing it.
I think using fakeredis to test is totally fine.
there is persistance across bot restarts with docker redis, of course.
just not across redis restarts
patent pivot
it's persistent, just not guaranteed persistent
crude gyro
well I think Mark means we have no volume for it.
patent pivot
ah right
crude gyro
so if we rebuild it all is lost
tawdry vapor
Yes that is what i mean
patent pivot
should make one then
tawdry vapor
If you restart the container data is lost
tawdry vapor
Youä're right, sorry for using poor terms
i meant if you delete the container and make it again
crude gyro
yes.
tawdry vapor
Like if you docker-compose down, which deletes the containers
patent pivot
ah right yeah
crude gyro
anyway I don't think a volume is really relevant
not even sure local redis docker compose is relevant?
since fakeredis totally is fine.
tawdry vapor
Well, I guess. But it was easy to add to docker-compose so it's there if someone wants it
crude gyro
yeah
tawdry vapor
We would have to document it though -.-
crude gyro
it was. it's fun to add a whole service with two lines of yaml
tawdry vapor
Or just say "use fakeredis. if you wanna use redis in docker compose you're on your own"
patent pivot
yep
crude gyro
the redis option is tricky and will not work on windows at all.
well.. it might. but it's not worth the trouble.
tawdry vapor
I don't want to have a broken redis in docker-compose though. I mean this regarding the password. you say it works if the bot runs via compose too which is weird
So I think we should still fix that
crude gyro
yeeeaah I'm not sure what that's all about.
hard for me to fix though because I am not having the problem
but if you figure it out, a fix would be very welcome
tawdry vapor
So how about password=password if password else None and calling it a day? Does anyone see a harm with this line?
tawdry vapor
The only issue I see if someone sets their redis password to an empty string, if it even supports that
But screw them if they do that
patent pivot
redis will support it but that would be silly
crude gyro
yeah. fuckem and their law
patent pivot
we could add a courtesy log
crude gyro
I reaaally don't see the point. nobody is ever gonna try to set up redis.. probably. at least not after we tell them they don't have to.
the thing I wish we had was local dev autodetect
so that we didn't need that use fakeredis config value
tawdry vapor
if constants.Redis.password == '':
log.info("Redis password was an empty string; setting it to None.")
password = None
else:
password = constants.Redis.password
sure sure. works for this guy right here.
tawdry vapor
And yeah, that fix works
crude gyro
cool.
patent pivot
nice
tawdry vapor
I will also add a comment explaining why we do this actually
crude gyro
I always say funky fresh
but I have realized recently
that in terms of odor, those kinda mean the opposite thing
and I don't know if that's okay
I am a bit offended.
hashtag gareths grumblings
now I'm the gareth
good god.
uhhhh
patent pivot
lmfao
tawdry vapor
a null value in yml?
crude gyro
it's not just null?
patent pivot
uhh
crude gyro
or none
patent pivot
no it's null and ~
>>> yaml.safe_load("test: null")
{'test': None}
>>> yaml.safe_load("test: ~")
{'test': None}
never seen tilde used like that. cool.
crude gyro
no, that's problematic.
tawdry vapor
Tell people to use null for pw in config
patent pivot
or just literally not setting a key
crude gyro
because it will fail the test
crude gyro
because it typechecks
patent pivot
sadface
crude gyro
and None is not str
tawdry vapor
Ok that makes sense
patent pivot
that is a cleaner solution imo
tawdry vapor
I didn't think about it
tawdry vapor
Well we can get around to that some time
patent pivot
yeah
tawdry vapor
We need to re do the entire thing honestly
I would at least like to see optional support though
Cause a re-work is gonna take even longer to happen
crude gyro
mvp optional is just telling the typechecks constant test that None is an acceptable type too
probably a oneliner
then missing env vars (which are None) will not fail tests, neither will literally setting null
which would be nice
crude gyro
since redis pw looks for env var, that means we can just not have that config in config.yml
and it will be None
pretty clean
tawdry vapor
Is it an issue with yaml parser or just our tests for it?
crude gyro
well we can't put a type like Optional[str] in the dataclass
it's not supported
or the NamedTuple.or whatever it is.
so if we wanna allow it, the tests will just have to permit it
tawdry vapor
Ok, so just a test issue
crude gyro
should be.
tawdry vapor
I was basically hoping I don't need to touch the metaclass
crude gyro
you most likely don't.
I very much doubt it.
it's not that scary though
I think it's something I wrote back in the day
or, at least that I designed
someone else probably committed it
maybe ELA
but that whole metaclass design is very oh hey, look, metaclasses wtf is this, look, I barely knew how they worked
so it can't be that scary
tawdry vapor
Alright I am looking at the test
I think I can fix it
or at the least, skip the test like we do with list and dict
crude gyro
🙏
tawdry vapor
I mean fixing the test rather than working around it by adding some borderline hack to set the password to None
tawdry vapor
`Kind of
if getattr(annotation, '_name', None) in ('Dict', 'List'):
self.skipTest("Cannot validate containers yet.")
So the ez fix is to add optional to that tuple
But I'm gonna see if I can actually validate optional
crude gyro
hnnnmmmmmmmmmmmm
tawdry vapor
I just need to figure out how to get the type inside the brackets
crude gyro
I think Optional will be rejected at a lower level
before tests even run
but that would be veeery nice.
I do think it's probably acceptable to just implicitly say that everything is optional, though
if we must
but actually supporting Optional would be nicest
crude gyro
oh okay
that's awesome
then we can actually add real support for it.
maybe
that'd be a killer fix.
100% an improvement
tawdry vapor
Oh yeah I forgot optional is just an alias for union with none
Glad I have a debugger to show me this
So I can add union support!! wow amazing
tawdry vapor
I wonder why the typing lib doesn't support isinstance checks for unions
And all attributes are private or even double underscored so
crude gyro
that would be nice.
tawdry vapor
This is a real pain to try to get the actual types out of the union
crude gyro
that's kind of a shitty interface that doesn't easily expose that
tawdry vapor
crude gyro
mmmaybe.
tawdry vapor
No I don't think they're gonna help
tawdry vapor
Oh neat
Thank you very much
I was just about to figure out how to access __args__ lmao
Like, with the name mangling shit
crude gyro
haha, yeah. figured they would have an easier way
found it in the typing docs
they have a whole bunch of helpful methods in there
fyi
tawdry vapor
I don't think I will add support for lists/dicts because it gets complicated if they are multi-dimensional
tawdry vapor
Sure, why not
crude gyro
but I'm not sure what a list or dict looks like in yaml either
i mean don't we have lots of lists
is it just that they're not typechecked?
tawdry vapor
I believe this is a list
modlog_blacklist:
- *ADMINS
- *ADMINS_VOICE
- *ATTACH_LOG
- *MESSAGE_LOG
- *MOD_LOG
- *STAFF_VOICE
It's a dict key that has a list as its value
crude gyro
yeah exactly
tawdry vapor
Well, I'm not sure how to check if a type annotation is one dimensional
Cause there are a bunch of different sequence types
crude gyro
check if any of its args are containers?
tawdry vapor
Well, if we limit ourselves to only list and dict then it's feasible
crude gyro
that's what I was thinking
tawdry vapor
I think using any other annotation would be wrong anyway since yaml cannot return that
crude gyro
yes.
so only lists and dicts that don't contain other lists and dicts.
not sure yaml really supports multidimensional lists either
so it's not that much of a cop-out
patent pivot
lol
tawdry vapor
Have to account for something like Union[List[int], Dict[str, int]]
And if we have Union[List[List[int], other shit] then I have to check in the union too that there aren't multidimensional containers
It took me long enough to realise there's probably a library for this
And sure enough
So we can just use that for a proper solution
Well maybe
I don't know if it will play well with our metaclass
Also, we this is designed for runtime while we want to check in tests
Maybe they have an internal api we can use in tests
pydantic is primarily a parsing library, not a validation library. Validation is a means to an end: building a model which conforms to the types and constraints provided.
In other words, pydantic guarantees the types and constraints of the output model, not the input data.
Ok then
For now I will just add optional support. I don't think trying to validate containers and such ourselves is wise
This is obnoxious they need to fix this https://youtrack.jetbrains.com/issue/PY-30425
crude gyro
not testing related,although I suppose you could use it for that too
it's for when you need to permanently monkeypatch some function inside of some package you're using
eternal owl
How can I switch to a branch which is on the main repo(pydis bot repo) when I am on the forked version? I used to work on the branch restricted_tags but after removal of direct access to contributors, I have to work on the forked version.
sullen phoenix
you'd need to do git fetch upstream <branch> and then checkout to it
assuming you have an upstream remote set to the main repo
eternal owl
okay
eternal owl
I think I should open a new PR?
?
updated the branch by pulling from master and also resolved a conflict(hopefully)
green oriole
The base branch on PyDis should be master though :)
gusty sonnet
Since it's a pretty minor conflict do you want me to resolve it for you @eternal owl
eternal owl
Sure @gusty sonnet, and thanks!
cold moon
About https://github.com/python-discord/bot/issues/717 , I think this should use Redis to storage usage counts. Only thing is that, is there any way to run script that fetch all audit log from this server and add this to redis and only once?
woeful thorn
There's no reason to do this with redis when we can do it on the backend just fine
crude gyro
yeah that's silly. if we want information about the otname usage we should solve it on the site
redis is for temporarily caching stuff
not for permanently storing the entire audit log
cold moon
I mean like name: count
crude gyro
yeah, no. you should not rely on redis data being permanent. that's what postgres is for
redis can be cleared at any point.
don't store stuff there if you cannot afford to lose it
cold moon
Okay
crude gyro
if we want the number of times a otname has been used, we can make a migration for the site that polls the audit log via the discord API and figures it out
and then we can store that information there
where it belongs
crude gyro
yes, to find the number of times an otname has ever been used
if we wanted that - it's mentioned in the issue
patent pivot
riiiiight
crude gyro
but it's still solvable with a simple migration and some discord api magic
patent pivot
audit logs will not work at all
we can only filter by user and action (Python, channel update) and there are hundreds of those a day from the help system
crude gyro
yeah, so it'll be hundreds of thousands of items
cold moon
Oh, and I just found that we are not able to get ALL Audit Logs. Only some months
patent pivot
yeah, audit logs just are too volatile for this sadly
crude gyro
but that's okay
it'd be a fun feature anyway if we just start counting from the day it gets merged
cold moon
Then we can start from 0
crude gyro
but I still think we should solve it on the site, not the bot
patent pivot
yeah
crude gyro
the site manages the otnames, makes no sense to put the data anywhere else.
patent pivot
should be added to off topic record
cold moon
I think only thing is announcement message that should be done in bot.
tough imp
where is the choice for the next-up names made, is it in bot? or does the site only send the 3 chosen names?
crude gyro
it's in the site.
crude gyro
the bot just asks for otnames
yes, it is trivial
make a new model to store otname metadata, add a migration for the new model, start storing data in it when we use the otname endpoints
done
cold moon
I have PR for fair selecting otnames https://github.com/python-discord/site/pull/348
patent pivot
can't we add a col to off topic names
cold moon
New column will be much easier
crude gyro
sure, if we just need count that's fine
creation date we might actually already have in the site I think
it's very hard to store things without getting that for free
Django generally likes to have it.
cold moon
Just we have to find way to fetch it.
crude gyro
hmm. I might be wrong
yeah okay
but then we'd just have to initialize it to todays date
and add real dates for all future otnames
which kinda sucks a bit
cold moon
😭 Maybe we should create new mixing for models that contains created and updated fields?
crude gyro
yeah, I actually don't hate that idea.
woeful thorn
There's only 590 names, time to get someone to search through chat history
cold moon
lmao
crude gyro
haha, just search for when they were first added
god damn, if only discord search had a real api
we could easily solve that problem
I mean, it sounds like a joke but it honestly wouldn't be that many if we just had like 6 people participating.
I could do 90 searches in like 20 minutes
stick all the datetimes into a google sheet.. we'd have it banged out in no-time.
crude gyro
hahaha
mellow hare
If they aren't picked that day they get their weight increased by 1, and if they were picked it drops to 0
But that's a lot of updating in the DB for something silly like this
crude gyro
I think you're talking about something different than us
possibly about rotating otnames
crude gyro
we're talking about backfilling creation dates
mellow hare
Ohhh okay
patent pivot
huh yeah, ot name table really does just have a name
crude gyro
yep
patent pivot
yeah
crude gyro
what do you think of ks's idea to introduce an abstract base class or a mixin or something to add creation date and updated date
mellow hare
I think I'm just obsessed with self balancing randomness.
And yeah, I like that idea as well
patent pivot
yeah that sounds good to me
crude gyro
I'm not sure about adding it to every single model or anything but at least to the otname one
mellow hare
Not impossible for us to write an int eval script to find the dates they were added
patent pivot
with most ORMs that's just what has been default, I'm surprised
how would we go about that?
crude gyro
actually I'm pretty sure that would be impossible
patent pivot
how thouggh
crude gyro
by what, searching the history of every single channel?
patent pivot
requesting the entire message history?
patent pivot
discord would ban the bot most likely
crude gyro
that would block the bot for days
mellow hare
Unless we pulled in Joe's bot
crude gyro
it's 8,5 million messages
crude gyro
I think if we really wanted to solve it, it'd be better to just ask 5 staffers to do it manually
I'd join.
boring manual labor while in a voice chat for half an hour
mellow hare
If I wasn't tied up with this database migration I could
crude gyro
and we'd be done
mellow hare
I guess I could when I'm at home
Would we just want dates or dates and time as well?
patent pivot
dates
green oriole
If everything is in one channel, a script would do it
patent pivot
It's not
green oriole
Or the bot logs perhaps?
mellow hare
Damn
green oriole
Do you keep them forever?
crude gyro
we don't keep logs that far back
patent pivot
If we had a number of staff members volunteer I could partition the OT name table
mellow hare
Swap server with guild and local with server
Send out the call to arms
I'm sure we've got plenty of board folks
crude gyro
they wouldn't be able to see most of the otname creations
they'd be in mods+ channels, a lot of the time.
sometimes even admins+
mellow hare
It'd have to be a group of admins then yeah. I'll join the crusade when I get home from work then
crude gyro
I'm just thinking out loud here, not suggesting we need to get started with this immediately
mellow hare
Fair enough
green oriole
We could have an approximation with the action log though
patent pivot
what action log
crude gyro
you mean the audit log?
green oriole
The thing logging name changes, is that the audit log?
crude gyro
nothing logs name changes. except the audit log, which doesn't store them forever.
patent pivot
that gives us usage, not insertion time
green oriole
That's why approximation
mellow hare
Sometimes it's just better to roll up our sleeves and just trudge through it
crude gyro
I do think we used to log the namechanges every night somewhere
green oriole
I thought you had a log for channel names changes
crude gyro
one of the logs has that
yeah #mod-log has these
we could get a rough count from that
on a subset of the names
around 96K messages in that channel.
green oriole
Well, I think the first usage time would be enough, wouldn't it? And you could scrap that with a bot
mellow hare
Entirely possible we have some that have never been used
patent pivot
first usage time isn't enough imo
mellow hare
^
patent pivot
i'm picking them at random from the DB and there are ones 2 months out
and that's a low figure
crude gyro
First otname change log was in 2018-11-23. in:mod-log after: 2018-11-23 has around 28K messages
patent pivot
I've got one here added in October 2019, it was first used in 27th February 2020
first usage won't cut it
crude gyro
not sure I care that it's inaccurate.
if we iterated those 28K to fetch first-time use, count, and then made everything else count 0 and created datetime.datetime.now(), we'd have some cheap, free seed data
mellow hare
What's the main purpose of having the creation dates?
patent pivot
I'd rather have no data than wildly inaccurate data
hardy gorge
yeah #mod-log has these
@crude gyro
That specific one you picked was a manual change I made when we hit 50k
just as an anecdote
green oriole
It could just be
Channel name changed to ot0-channel-name. This named has been picked 42 times and was first used on Jan 1st 1970.
Instead
patent pivot
wait no
we suppress anything else
that only showed up because the bot didn't do it, right?
hardy gorge
The bot's embeds were always glitchy
crude gyro
I think the count data would be decent from iterating like that.
hardy gorge
We had a time when the bot changed the channel names like 20 times in a second because we had those timing issues
And now, it typically only posts one out of three channel name changes
crude gyro
and @green oriole makes a pretty good point about how "was first used on <date>" being roughly just as interesting as when it was created.
crude gyro
We had a time when the bot changed the channel names like 20 times in a second because we had those timing issues
@hardy gorge
this is true, that makes the count data kinda fucked, actually
patent pivot
let me check the ratelimits
crude gyro
yeah, I think that glitch ves mentioned, which was there for a long time, is a dealbreaker
so fuck iterating that channel
hardy gorge
and the fact that it typically only posts one of the three changes per day now
and I don't know why
crude gyro
we could just not show any date
or make creation date nullable
and show it only for ones that have it?
hardy gorge
yes
patent pivot
ratelimits are too mad for iteration
crude gyro
we could still do count, just starting at 0
patent pivot
yeah
hardy gorge
It's a fun thing, but it's not mission critical to have historical records
Feature-introduction-forward seems fine to me
crude gyro
so, feature forward and nullable ctime, have the bot only display ctime if it's there. and no dumb manual search party
waste of manpower
or skip ctime entirely? what do you think
green oriole
Well, I don't see why not for have it for newer otn, it is basically free
crude gyro
because people would go "why is it so inconsistent"
but I don't really know either, I don't mind either way
patent pivot
delete all off topic names 😎
crude gyro
@cold moon I think we should get your off-topic derandomizer PR merged, and then we can add this creation time and count stuff after that - maybe you'd like to handle that too?
I've left a full review on https://github.com/python-discord/site/pull/348 for you so we can get it merged.
hardy gorge
I don't really care for or having anything against having creation times
I guess it could be fun
hardy gorge
yeah sounds fine
cold moon
OK, I'm gonna make fixes tomorrow. Gonna go sleep soon
crude gyro
sounds good.
cold moon
@crude gyro I addressed reviews
crude gyro
nice. will take a look soonish.
hardy gorge
Hello <@&295488872404484098>!
We've just merged an important update to @stable mountain that includes some changes that you need to be aware of if you're currently working on or planning to work on the bot.
-
The merge includes a re-lock of the dependencies, so be sure to run
pipenv sync --devif that's needed for your setup. Some of our linting dependencies have been updated in this relock and added some additional linting rules (such as not allowing f-strings without placeholders in them). Be sure to run pre-commit and/or lint with the new settings in place. -
The update introduced support for Redis, an in-memory datastructure store and caching system. It is not necessary to run Redis locally (although, if you're running the bot using the docker-compose that will be automatically taken care of it); you can use "fakeredis" instead. To do this, you have to add these lines to your
config.yml:
# You probably already have a `bot` section
# add a `redis` section to the existing `bot`
# section.
bot:
redis:
use_fakeredis: true
- if you need to use the redis caching database for a feature, please take care to read the docstring of the RedisCache class in
bot.utils.redis_cache. It explains how you can use it. Do make sure that you actually need it for your feature and if it's the appropriate place for your data. For data of greater value, the Postgresql database remains the database to use. Don't hesitate to contact a PyDis Core Dev if you're confused.
steep saffron
cold moon
https://github.com/python-discord/bot/issues/856 I think this should use Redis for storage of every user last display name alert sending time.
patent pivot
I don't understand why we need the last display name
green oriole
I think it is about the cooldown between notifs
cold moon
Yep Akarys correct
patent pivot
We don't need to store names then
cold moon
userID: last_sent_notification_iso
patent pivot
I've just rewritten the privacy policy to try avoid storing personal data where not needed, and I don't think this is needed
right, that is fine
green oriole
You juset need a cached dict id -> last_time thanks internet
patent pivot
as long as there is no PII I'm fine
cold moon
But should this use on_message or on_member_update?
patent pivot
on message as lemon says
patent pivot
I see your reasoning for on_member_update but we have no interest in moderating users that are not active in the community
cold moon
I just mean can I do this or I have to wait until label get removed?
patent pivot
I'll assign it to you now, if we have implementation issues we can put them on the PR
cold moon
Thanks
bronze hare
I don't know if this is previously considered or not, but it would be cool to not to post mails if they start with Re: to #mailing-lists
patent pivot
@bronze hare yeah I think it's proposed, not sure if we have an issue yet
but if anyone wants to add it feel free
solemn cliff
I couldn't figure out how to do this issue - I think it's too far beyond my current level. I'll look for another more beginner-friendly one - could it be unassigned from me? Thanks :)
https://github.com/python-discord/seasonalbot/issues/174
cold moon
Nice
tough imp
@solemn cliff sorry for the delayed response - looks like you were already unassigned from the issue by another core dev, so no worries
however, if there is anything you need help with, you should feel free to reach out
we can help you get started or answer any concerns you may have
cold moon
@tough imp There will be not everyone ping spam in mod alerts because this don't mention everyone
tough imp
the problem isn't necessarily the ping, but let's imagine that a spambot comes in and trips the displayname filter
if we begin processing 10 messages before the first finishes and has a chance to write to the cache, the subsequent events won't know that they shouldn't send the alert
and the mod alert channel will get spammed by the bot
whats even worse is that this can also get our own bot ratelimited, which makes it unable to continue responding to an attack
for these reasons I think we should be careful
cold moon
Okay, so I should put whole this function to asyncio.Lock?
solemn cliff
@solemn cliff sorry for the delayed response - looks like you were already unassigned from the issue by another core dev, so no worries
alright thanks!
brazen charm
https://github.com/python-discord/bot/pull/875 got overwritten by #866; not sure what the proper way to fix this would be so sending it here (tried cherry picking the original commit for a pr but it looked weird... and not sure)
tawdry vapor
Just create a new commit.
If you really wanna figure out how to use mine then go for it but it doesn't really matter
@gusty sonnet did it show any conflicts when you merged 866?
I didn't see rohan push anything to fix conflicts
brazen charm
Alright, opened a pr with from the branch I had the cherry pick in
hardy gorge
I think Shira manually resolved the conflict in that PR
It's in the latest merge commit
brazen charm
Managed to mess up the message in a merge commit after battling with pycharm and git for a bit, should I amend that? https://github.com/python-discord/bot/pull/927/commits/ed9e1de6d9b47fe616f00e5fcf324e4c6574a49e
brazen charm
Well, nothing to see here
gusty sonnet
Uh oh what did I do haha
tawdry vapor
@cold moon FYI I will have to approve this tomorrow cause I still need to test locally and it's already late here
cold moon
ok
lament totem
Hi , i had a query (Related to issue 377 )
i have the img url so :
trying to use
async with self.bot.http_session.get(url) as r:
json_data = await r.json()
my response is not gonna be json , can i use "r.content" ?
tried to do the above but i get
im_data = await r.content
TypeError: object StreamReader can't be used in 'await' expression
You should get a ClientResponse object and you can use the methods/attributes it provides: https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.ClientResponse
What kind of content do you expect? Bytes? a string?
I think you're requesting an image (going by the "img url" line) and you probably need the bytes for that
You can read those in with https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.ClientResponse.read
lament totem
@hardy gorge Sure ! will check , Expecting Bytes.
lament totem
Hi ,
The issue 377, i believe i have finished the proposed implementation.
So , what needs to be done next to merge ?
patent pivot
@lament totem you'll need to open a pull request
https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request
https://www.youtube.com/watch?v=rgbCcBNZcdQ
lament totem
Thanks !
cold moon
@tawdry vapor What GitHub logo I should use?
cold moon
OK, thanks
rocky bloom
should the !shhh/silence command also remove reaction perms if the channel has it
woeful thorn
lament totem
Hi All,
I wanted to know how this is implemented or to be implemented -
Your commit will be rejected by the build server if it fails to lint.
This is in the rules (5th Point) ! so just curious how you guys are doing it !
is it a like a custom setup that needs to be done in Git ? or what is it ?
molten bough
It's done using continuous integration
One of the tools used is called flake8 and it's in charge of checking over your code - we call that a linter
When you push code to the repository (or a pull request), flake8 is automatically run against it using the CI system they have set up
Which could be Azure pipelines or GitHub actions depending on the project I think
brazen charm
It's remarkable how inconsistent embeds are... working on the second point in https://github.com/python-discord/bot/issues/878
Making the relative hyperlinks in the doc output actually work ends up not displaying inline codeblock on mobile, but hyperlinked `text` shows up. Do you think that looks alright or should codeblocks just be stripped away when the result tag is a hyperlink since that doesn't need them anyway?
Ends up like this, desktop client works fine
cold moon
@tawdry vapor Source command reviews addressed
cold island
@tawdry vapor I'm at this point now https://pythondiscord.com/pages/contributing/hosts-file/#windows-macos
But no such command. Where was I supposed to run it? inside the container? that's a later step though...
brazen charm
I think the docker-machine comes with the toolbox if you got it through that
but you can try the 192.168.99.100 ip (should return an error page when you go to the socket you specified)
hardy gorge
ModMail, as a user, is probably not in your cache and mentions in an embeds won't fill the cache automatically, unlike mentions in a regular message. At least, that's what I think it is.
Maybe @patent pivot has another perspective
crude gyro
yeah it's totally a caching thing
Discord on mobile is notorious for showing @invalid-user at every turn
particularly inside of embeds
celest charm
@patent pivot works as intended 😎
patent pivot
nice
celest charm
I still think we should keep regex. It has cool features not present in re, and it is better at handling catastrophic backtracking (it has a timeout feature!).
dull jetty
cloudflare has left the chat
cold moon
Yes, this is not so plain anymore
green oriole
It would be nice to include some regex examples too, from official ones, like the email regex
celest charm
@patent pivot I completely forgot they existed haha
patent pivot
named groups would be pretty cool, I try advise everyone use them where possible
green oriole
Noice
patent pivot
doesn't support everything though since no % or whatever and I got lazy and handed it off to someone else to fix lol
green oriole
That reminds me of my 230 chars datetime regex haha
patent pivot
oh akarys why
green oriole
The code jam qualifier haha
celest charm
docker-compose build just froze my laptop, thank you docker-compose
patent pivot
lol
green oriole
https://github.com/Akarys42/code-jam-6-qualifier/blob/master/qualifier.py#L6 the bad guy is here hehe
celest charm
Maybe you could use the VERBOSE flag?
green oriole
I commented every line >.>
green oriole
Yeah, but there was a reason why I couldn't use it, I don't remember which tho
I think there was something about spaces or so
celest charm
!e
import re
r = re.compile(r"""
(
hello # greeting
| world # the entire world
| \d+ # a number
| a[ ]+b # a and b separated a bunch of spaces
)+
""",
flags=re.VERBOSE
)
print(r.match("hello123worlda b"))
@celest charm :white_check_mark: Your eval job has completed with return code 0.
<re.Match object; span=(0, 20), match='hello123worlda b'>
That's ugly though
celest charm
I don't see how this is worse than
r = re.compile(
(
r'('
r' hello' # greeting
r' | world' # the entire world
r' | \d+ ' # a number
r' | a[ ]+b' # a and b separated a bunch of spaces
r')+'
),
flags=re.VERBOSE
)
most editors will highlight the regex properly
sublime
vscode
green oriole
Ah yeah, because of the closure you need the []
celest charm
That's just because verbose regex ignores whitespace, and you have to mark specially
or you can use \s for any whitespace character
I usually put [ ] instead of because can be easy to miss
clever wraith
I'm still not 100% sure I read of a satisfying reason as to why this isn't being executed in snekbox, can anyone clarify?
patent pivot
The original concern was catastrophic backtracking but the regex module seems to prevent that and has timeouts
however I do still somewhat worry about things
I think with the refocusing of snekbox to contain multiple evaluation systems regex is something we could look into
green oriole
Does it support codeblocks too btw?
Regex compiling is safe AFAIK, if the timeout is set
patent pivot
yeah, with catastrophic backtracking being prevented it does seem pretty safe
clever wraith
I'm personally on the opinion that any unrestricted user input to a state engine is inherently unsafe, and should be sandboxed
but that's just me and my paranoia
patent pivot
I do share the feeling
brazen charm
Wouldn't be hard to just have some template code where the regex would be placed that would be sent to the current snekbox api, bit slower than running it directly but eval is fairly fast
celest charm
Like, call the !e command?
green oriole
Maybe this would be a good use case for a backtracking-free engine, like RE2 by Google
Yup
brazen charm
yeah just construct the code to run from input and then send it to snekbox like !e does
patent pivot
I don't hate that idea
green oriole
Bindings for the RE2 engine, I think it might be worth looking into this, it has a completely linear execution time https://pypi.org/project/re2/
patent pivot
I'm not sure we should use re2, if we move to snekbox I don't even think we should use regex
cold moon
patent pivot
I think that the usage of this will be demonstrating re most of the time, and sometimes it may be useful to demonstrate the pitfalls of the module
mellow hare
@celest charm Looking through the PyPi article, it's saying that if there's something that the re2 library doesn't handle, it falls back to the re lib
So it's sort of compatible
cold moon
I like RE2 idea
green oriole
I don't even think we should use regex
@patent pivot ... How?
patent pivot
the regex module
clever wraith
I think that the usage of this will be demonstrating
remost of the time, and sometimes it may be useful to demonstrate the pitfalls of the module
@patent pivot can't react in this channel, but i agree with this
patent pivot
@green oriole I'm proposing if we move to snekbox we should move back to re, because I don't care about catastrophic backtracking in snekbox
brazen charm
I think it should be done in snekbox for the ability to use re and it's full behaviour
patent pivot
yep
green oriole
Ah, the current implementation doesn't the standard stdlib implementation
patent pivot
@crude gyro the proposed regex commands currently run regex on the bot
now obviously we've had our runins with unsafe regex before
crude gyro
which regex commands
patent pivot
nix's codeblock stuff
@celest charm has PRd some regex commands
to demonstrate regular expressions
mellow hare
Getting the pr link