Should make testing easier as well since you haven't got a custom solution to test.
#dev-contrib
hardy gorge
eternal owl
can you link me to the markdown package? there are so many
valid quest
hardy gorge
@eternal owl Check which one django-wiki uses. That one should also be already installed (as a dep of wiki), but if we're going to use it directly, we should add it to our packages as well. I think the name is simply Markdown, but please check.
green oriole
@valid quest I think in case the tag resolving fails, you should try to match the command
valid quest
I tried that, (catching instead of ignoring it)
I will try to see why it didn't work
valid quest
@green oriole I can see that nothing is really raised when the tag isn't found, maybe we could return or raise something, or just get the tag before calling the command
I managed to get a solution, but it seems a bit over (getting the cog and its cache to check if the tag exists)
It is the only way that i can think of, since
I can see that nothing is really raised when the tag isn't found
green oriole
Are you working strait of master? There should be a fuzzy matching of the tags
valid quest
There is
So when a command is misspelled, the bot treats it like a tag
iirc
The idea was to suggest better usage of a misspelled command, but the tag system is colliding with it
green oriole
Can't you add an else line 140 in bot/cogs/tag.py invoking a command matching or raising an error?
valid quest
Ill take a look
@green oriole Do you have an idea how to get all the commands into a string or a list?
I had this simple solution -
raw_commands = [(cmd.name, *cmd.aliases) for cmd in self.bot.walk_commands()]
But it doesn't seem well fitted for this case
Maybe i'll join it
valid quest
I got a suggestion to use bot.walk_commands()
I used commands before
I found the most similar command using similar_command_name = process(command_name, raw_commands, 1)
raw_commands needs to be a list and not a list of tuples
But it doesn't seem well fitted for this case
raw_commands = [c for data in commands for c in data] i will consider this
green oriole
Don't forget to comment it then ^^
valid quest
Yeah
Im trying to find a good implemention right now
I will try to add it to the tags get if this won't work
Also, does Tags._fuzzy_search take a string generally? do i have to format it in order to find the similar command?
green oriole
I think you should use _get_suggestions instead
_fuzzy_search just returns the match score between both strings
valid quest
Yeah, seems like it, thanks !
I will only need the top suggestion, so ill cut the results
oh wait, _get_suggestions looks on the tag list
I meant the commands
green oriole
Yes, you can just feed it the command list and maybe adapt the variable names
valid quest
I will use fuzzywuzzy.process seems the best for this case
mellow hare
Of course
crude gyro
Thanks @valid quest
I've added you to the Contributors team, thanks for the time you've invested in working on our repos lately!
valid quest
Oh thanks! and np 😄
Also, to let you guys know, this feature is not suggesting aliases, since trying to find a match with aliases is not accurate and slower, this was the plan, but maybe in future releases we will find a way to achieve this
cold moon
Can any core dev review https://github.com/python-discord/seasonalbot/issues/354 ?
green oriole
Sorry wrong button
Anyway sir @valid quest
./bot/cogs/tags.py:15:1 I100 Import statements are in the wrong order. 'from fuzzywuzzy import process' should be before 'from bot.pagination import LinePaginator' and in a different group.Did you installed the precommit? ^^
Guys, when you're working on the bot, you need to stop and restart the container everytime you change something, right?
woeful thorn
Or just reload the cog
green oriole
Hmm, what command is that?
hardy gorge
extensions reload cogname
green oriole
Thanks!
valid quest
Wait, did I do something wrong?
green oriole
You apparently did 😄
brazen charm
imports again
green oriole
Always those freakin' imports
brazen charm
they're supposed to all be in alphabetical order (along with the imported names) in 3 groups: stdlib, 3rd party and then local to the app
cold moon
Quick question: I market all sco1 reviews resolved, but GitHub still say there is still change requests: https://github.com/python-discord/seasonalbot/pull/350
green oriole
Yes, he have to say that it is actually resolved
cold moon
oh
brazen charm
They have to set it to to resolved, nothing's stopping you from resolving a conversation without making the required change. The changes can also be requested in the review comment instead of a code comment
green oriole
@hardy gorge @woeful thorn I don't really like to ask for revieing, but.. December 15 😄 https://github.com/python-discord/bot/pull/617
woeful thorn
Any core developer can dismiss stale reviews
woeful thorn
If I'm busy, pinging me isn't going to make me less busy. I do reviews when I have the time
green oriole
No worries 😄
Yes, but I just wanted to remind you about this one, I'd have understand if you have like, forgot about it
gusty sonnet
Hmm, do you want me to go ahead with your review on the movie command @woeful thorn it looks solid for now
woeful thorn
If the comments have been addressed and you think it's fine, then yes
I've been busy
gusty sonnet
I'll go ahead then, it has been addressed and the key was moved to constants.py
Hmm, how can we add env to the master?
green oriole
An admin have to, if it is an API key afaik
woeful thorn
In general, ask in the core dev channel. There should already be a TMBD key in the environment, the call is just hardcoded in the Halloween cog instead of the constants file
gusty sonnet
Ah! Thank you, I'll continue over there
cold moon
Thanks. My first contribution to PyDis...
green oriole
\o/
cold moon
Looks like there is some issue with TMDB API key. I tried command in #sir-lancebot-playground but this don't work.
gusty sonnet
I'll ask in core channel
gusty sonnet
You cant except without a try, what are you trying
Right, we didnt merge the pagination fix
green oriole
@gusty sonnet I'm trying very hard, but I really can't get it https://github.com/python-discord/bot/pull/710#discussion_r377133360 the except block is supposed to be called when exiting the eval loop, we just want to clear one last time just to be safe
gusty sonnet
Right now it's like this py try: ... await ctx.message.clear_reactions() await response.delete() except asyncio.TimeoutError: await ctx.message.clear_reactions() return
So I was suggesting doing it like this
try:
...
await response.delete()
except asyncio.TimeoutError:
return
await ctx.message.clear_reactions()```But I guess you are right, that return will mess up the final reactions clear
green oriole
Okay, so I can keep it like this?
gusty sonnet
Yep, I'll mark it as resolve and explain why
cold moon
Can anyone investigate (I have to go music school): After Pagination cog fix PR merge, at least for .movies command images not showing up.
dusky shoreBOT
woeful thorn
gusty sonnet
discord pls
cold moon
So would https://github.com/python-discord/seasonalbot/issues/354 get approved now? I just want new task... 😅
patent pivot
@cold moon I've approved it
cold moon
Thanks 😍
brazen charm
Take the commit thing I talked about into consideration when making that.
also mentioned in contributing point 6 https://github.com/python-discord/seasonalbot/blob/master/CONTRIBUTING.md
cold moon
Okay
valid quest
@tawdry vapor Hey mark, i have managed to get a solution for this case
We can raise a simple exception to be catched in the handler, since invoke isn't returning anything
If that solution is ok for you, else, i will be waiting for your migrations
It is tested, i also switched to difflib, it is better for that case
tawdry vapor
I think it makes more sense to invoke a function instead of a command to get the tags.
valid quest
We still invoke it, just raising info if a tag hasn't been found or sent, so we will know that the command wasn't intended to be a tag, and we will send a suggestion
Oh, we can also just set a field of the bot, since invoke is a coroutine
But there are better solutions, i guess i will wait for your changes
tawdry vapor
My changes don't really touch the way it's done. It's more of a refactor
I mean I think it's cleaner to call a normal function which returns a Boolean than to invoke and rely on exceptions
tawdry vapor
That's why you shouldn't use invoke
Just call a regular function...
Not a command
valid quest
Yeah, i didn't touch that
That sounds better
The implemention before used invoke, i thought its needed
tawdry vapor
It may be good enough to get the call back from the command object
And call that
If not then move the majority of the code out of the command and into a regular function
tawdry vapor
Yes, you can still use can_run
valid quest
Its not using the d.py decorators
Aight
I will do that tommorow
I also implemented this to suggest aliases
clever wraith
yes
molten bough
yes?
clever wraith
chat moved from #community-meta
molten bough
link that PR pls
clever wraith
clever wraith
16 minutes
molten bough
Yeah, no build triggered
green oriole
Try to resolve the merge conflict, it should re-trigger the CI
clever wraith
I just did that only manually
glass pecan
its still showing conflicts
green oriole
Are you still commiting via the web interface?
molten bough
you can't really manually resolve a merge conflict
you need to actually do the merge and resolve as part of it
if you just updated the files to match then they'll still conflict
clever wraith
ahhhhh
eternal owl
I followed this website to render markdown @hardy gorge
https://www.imzjy.com/blog/2018-05-20-render-the-markdown-in-django
can any mod get me the embed for !tags class from the db
hardy gorge
valid quest
Hey @gusty sonnet , do you still want to make changes to the pagination cog?
gusty sonnet
Personally I dont, but I see there are parts that can be better
valid quest
Ok, ill see if i can implement something and update you
gusty sonnet
Okay
clever wraith
ok let me ask you a question
so there is usage of DM disabled error message , should i made it in the bot error handler ? so that whenever i need it i would raise it and it will handeled by it ?
is it even possible ?
clever wraith
so ok
for example the bookmark command it needs person to enable direct messages right
but if it disabled we send an error message
hardy gorge
Right, so you're talking about the bookmark command
clever wraith
can we add it in error_handler , so that it invokes
so discord.Forbidden will raise and send that error embed
instead me writing error embed again and again
green oriole
Why would you need to write the error embed multiple times?
clever wraith
I actually can rename it to error_embed but if someone else need it in future ? it will work automatically
eternal owl
https://github.com/python-discord/site/pull/331
can anyone tell my why the checks failed
hardy gorge
@clever wraith Do you know how the minesweeper command handles disabled DMs?
brazen charm
eternal owl
I cant understand anything in that
I already saw that part thats why I am asking here
brazen charm
The lines listed there aren't covered by the tests iirc
eternal owl
green oriole
Missing coverage I think
clever wraith
#[warning]No code coverage results were found to publish.
eternal owl
I dont know what that means
green oriole
You have to run the test first @clever wraith
hardy gorge
That's not the issue; the issue is that we require 100% coverage in our test suite on these type of features
clever wraith
its not for me @green oriole
green oriole
You need to have every single branch ran during test run, and some are missing @eternal owl
Where do you see that AG?
clever wraith
hardy gorge
It's in azure; the coverage reports are not published
I'll look into it later; it's not critical
eternal owl
So do I have to do anything?
clever wraith
@hardy gorge I can't find the handle for discord.Forbidden
woeful thorn
You need to write tests for your code
green oriole
It is pretty straightforward
We have some link at the bottom of the bot test readme
clever wraith
there is no check for that in minesweeper @hardy gorge
hardy gorge
Okay, so there's not
clever wraith
there is no try: block in entire cog
hardy gorge
I think the best way forward is to have a central feature that alerts a user that a certain command requires DMs enabled
A try-block should not be necessary; we should probably have some kind of error handler that does it
If we let it propagate and have a central error handler for it, we can handle it in a generic way
clever wraith
if isinstance(error, errors.Forbidden):
await ctx.send(embed=self.error_embed(" Please enable your DMs to receive the bookmark.", NEGATIVE_REPLIES))
return
ehh ?
ok replave bookmark with message
hardy gorge
"The command you're trying to use requires you to enable Direct Messages" or something
green oriole
Something like this? https://github.com/python-discord/bot/blob/master/bot/cogs/error_handler.py
hardy gorge
Right, so, let's do this in an another way, @clever wraith
clever wraith
if isinstance(error, errors.Forbidden):
await ctx.send(
embed=self.error_embed(
"The command you're trying to use requires you to enable Direct Messages.",
NEGATIVE_REPLIES
)
)
return
You also need to check if the error is because you actually tried to send a DM
hardy gorge
There are more error handlers that can be centralized, so we could open an issue to centralize it instead of adding one specific handler
green oriole
Not a misconfigured channel id for example
clever wraith
make our own error ?
green oriole
No no, just a bunch of if/else actually
There are some metadata stored in the exception
Including the channel ID, and if it is None, it means that it was a DM iirc
clever wraith
dm's channel id is user id
clever wraith
let me recreate the error 1 sec
woeful thorn
It’s not the user id
clever wraith
File "/home/ag/.local/share/virtualenvs/seasonalbot-0Ba-uPuk/lib/python3.8/site-packages/discord/ext/commands/core.py", line 79, in wrapped
ret = await coro(*args, **kwargs)
File "/home/ag/PycharmProjects/bookmark-re-discord/seasonalbot/bot/seasons/evergreen/bookmark.py", line 58, in bookmark
await ctx.author.send(embed=embed)
File "/home/ag/.local/share/virtualenvs/seasonalbot-0Ba-uPuk/lib/python3.8/site-packages/discord/abc.py", line 823, in send
data = await state.http.send_message(channel.id, content, tts=tts, embed=embed, nonce=nonce)
File "/home/ag/.local/share/virtualenvs/seasonalbot-0Ba-uPuk/lib/python3.8/site-packages/discord/http.py", line 218, in request
raise Forbidden(r, data)
discord.errors.Forbidden: 403 FORBIDDEN (error code: 50007): Cannot send messages to this user
@green oriole
good thing that error_handler worked
green oriole
You are playing a dangerous game, seasonalbot runs on 3.7 :D
clever wraith
@hardy gorge I think for now i am gonna add this to error_handler , and push up the bookmark PR , with it usage.
green oriole
Anyway, you just used a try/except?
clever wraith
this one is handled by inbuilt error_handler tho ^^
otherwise previously i used try/except
only other place i could see it in use is advent of code . with this
except discord.errors.Forbidden:
log.debug(f"{author.name} ({author.id}) has disabled DMs from server members")
await ctx.send(f":x: {author.mention}, please (temporarily) enable DMs to receive the join code")
Or should i make a issue for error_handler modification ?
woeful thorn
Excepting Forbidden in the error handler is too broad
clever wraith
if error.code == 50007
clever wraith
ok so if i use ibuilt error_handlers
it returns
I don't want it to return
if i don't add return it throws error in console but does the work what handler tells
hardy gorge
What returns? And why don't you want that?
clever wraith
if it return it breaks the loop for everyone else.
while True:
try:
reaction, user = await self.bot.wait_for("reaction_add", timeout=10.0, check=check)
except asyncio.TimeoutError:
await ctx.message.clear_reactions()
return
log.info(f"{user.name} bookmarked {target_message.jump_url} with title '{title}' from '{ctx.author}'.")
sent_person.add(user)
this loop
valid quest
Hey mark, if you are available, i would love to hear some ideas
tags_cog = self.bot.get_cog("Tags")
sent = await tags_cog.get_command(ctx, command_name)```I tried invoking the command manually , without invoke since it doesn't return anything
I think i got something, ping me if you got another idea
valid quest
@tawdry vapor Ive just tested a few of the ways you suggested me to use, neither of them works
invoke - not returning anything
Command.__call__ blocking the event loop somehow
Raising worked, we can also set a bot field
I would love to hear what you suggest, i also moved the implemention to the error_handler and added alias command suggestion
tawdry vapor
tawdry vapor
That's how the function corresponding to the command can be accessed
valid quest
So when the command is called. the coro is also dispatched
valid quest
Sure
tawdry vapor
I think that's basically what invoke() does anyway
valid quest
I already know that, but i don't see how we are going to return a value using this
invoke doesn't return anything
tawdry vapor
@commands.command()
def mycmd(self):
...
return True
ret_val = await get_command("mycmd").callback(...)
Oh, I for some reason thought the callback was added to the arguments in the command decorator
valid quest
Gotcha, that's a good idea
I will update you
Should i use an Enum, or True and Falses?
I already have an enum set
For the tag state, but seems a little overkill
tawdry vapor
If it doesn't work you can just move it out of a command like ```py
async def do_actual_work(self):
...
@commands.command()
async def mycmd(self):
await self.do_actual_work()
error_handler
await cog.do_actual_work()
yeah, that's what i did
Got stuck somehow
@commands.command()
def mycmd(self):
...
return True
ret_val = await get_command("mycmd").callback(...)
@tawdry vapor This seems perfect
I will try, sec
tawdry vapor
Booleans sound fine
It's kind of awkward to document that though since it will show up in the help info, which is info irrelevant to anyone but the bot devs
By "that" I mean documenting the fact that the command returns something
Maybe it is better after all to move it to a separate function 🤷♂️
valid quest
ok
sent = await tags_get_command.callback(ctx, ctx.invoked_with)
Like so?
Seems ok
tawdry vapor
Yeah I think
valid quest
Its getting stuck again
Why tho
Aight, found it
Still stuck, yikes
sent = await tags_get_command.callback(tags_get_command.cog, ctx, ctx.invoked_with)
We also need to pass self
Weird, why is it still stuck
@tawdry vapor Ping me when you are free to work on this, the code is getting stuck whenever im calling await tags_cog._get_command(ctx, command_name)
or await tags_get_command.callback(tags_get_command.cog, ctx, ctx.invoked_with)
valid quest
Nope
Maybe it is better after all to move it to a separate function 🤷♂️
Should i push the current changes?
tawdry vapor
Yeah sure
Remove the * from _get_command args and also delete the debug log on the first line in the function. Then it works.
valid quest
Great
Also why was the implemention using the consume_rest e.g *?
I didn't get it also
Oh, for a casual invoke?
tawdry vapor
I suppose in case tags have spaces in the name
valid quest
Yeah
tawdry vapor
It's fine to leave it in the command
valid quest
Yep
Also, is difflib fine for you? @tawdry vapor
We can also use levenshtein, but i don't see why currently
valid quest
Yeah, it is ok
@tawdry vapor I think its ready for merge, let me know what you think, also, should we suggest a few options or just the closest?
Only closest matches are suggested
tawdry vapor
I'd say only 1 suggestion but you could ask the issue's author
valid quest
The actual name of get_command isn't so suitable, since it doesn't really describe what the function does
But for now, ill keep it that way
clever wraith
that looks so good
crude gyro
@valid quest looks like it works well. I don't like the way the suggestion is presented, though.
it should be more in line with how the other errors are presented.
green oriole
Maybe also showing the syntax of the command, or maybe only the first one can be quite useful too
crude gyro
does it retain the arguments you pass it in the suggestion?
so if I write !muet everyone forever will it suggest !mute or !mute everyone forever ?
eternal owl
Maybe the bot can suggest !mute everyone forver and then add two reactions for yes or no, if the user reacts yes then the command will be executed.
gusty sonnet
iirc we've discussed about this, and it should show the corrected command for copy paste only, so it can be reviewed as well
It is working well
crude gyro
alright, good
I do not like the style, I think it should be in an embed with a 
and with the command inside code backticks or something.
otherwise that looks good
valid quest
@crude gyro it will correct the command with your arguments inserted
I will fix the style as soon as I get home 👍
@eternal owl I think this is a little overkill, but I will consider this, I also thought to add a 
emoji for instant message deletion
We can also make a general reaction event handler ( if not exists) to handle this kind of reactions
hardy gorge
No, we don't want a reaction interface for it
Especially not for moderation commands
A wrong mouse click should not have the power to ban someone
That's why we opted for the copy-paste ready format
valid quest
Ok, what about the emoji?
hardy gorge
It think it adds unnecessary vertical space to an otherwise very small embed (it could be just one-line: "did you mean: !command arg1 arg2")
valid quest
Ok, @eternal owl, you got an answer
@hardy gorge I will add the embed as I get home
Will also do a little cleanup and testing
valid quest
@crude gyro Hey lemon, is the question mark emoji you suggested to add is in the bot config? i can't seem to find it
This one
Oh it is, just as a link <"https://cdn.discordapp.com/emojis/512367613339369475.png">
Should i add it as an emoji?
molten bough
You can't use an emoji in embed headers
well, not a custom emoji anyway
has to be used as an image
valid quest
I planned using it in the description
Yeah, setting the author icon to it seems ok
@crude gyro Hey lemon, is this the wanted format?
Should i add a codeblock?
Yeah, it does look better
But it does not display mentions well
crude gyro
hmm. that's true about the mentions
gusty sonnet
Or you can just bold the command after you correct it to emphasize on it
The rest can stay the same
Like:
Did you mean:
!mute @gusty sonnet 50y potato as heck
valid quest
Up to lemon, looks good for me
mellow hare
Why so starchy
valid quest
Why is my lint saying every single file has no return type annotation when it does
Weird stuff
./bot/cogs/tags.py:174:27: TYP101 Missing type annotation for self in method
flake8 returnes no errors
I pulled from master
patent pivot
hmmmm
we disable that in tox.ini
[flake8]
max-line-length=120
docstring-convention=all
import-order-style=pycharm
application_import_names=bot,tests
exclude=.cache,.venv,constants.py
ignore=
B311,W503,E226,S311,T000
# Missing Docstrings
D100,D104,D105,D107,
# Docstring Whitespace
D203,D212,D214,D215,
# Docstring Quotes
D301,D302,
# Docstring Content
D400,D401,D402,D404,D405,D406,D407,D408,D409,D410,D411,D412,D413,D414,D416,D417
# Type Annotations
TYP002,TYP003,TYP101,TYP102,TYP204,TYP206
per-file-ignores=tests/*:D,TYP
yeah, it's probably because your linter isn't using our toxfile
valid quest
Yeah, i will update it
Thanks
@patent pivot it says it cannot accept , in
ignore=
B311,W503,E226,S311,T000```that's the standard syntax for it, how are you loading it?
valid quest
I ran pipenv sync
patent pivot
and it gave you that?
valid quest
Yeah
patent pivot
why would it check the tox.ini on pipenv sync whaaaaat
patent pivot
can you show me the full output of pipenv sync
put it on https://paste.seph.club/ if it won't fit
molten bough
yeah, check line 56 of the pipfile
patent pivot
but there is no line 56
brazen charm
is the bot on 3.8 yet?
molten bough
It's not the tox.ini
patent pivot
don't put that stuff in the pipfile
yeah the tox.ini is what flake8 reads from to get it's settings
Pipfile is for pipenv
flake8 won't look at your pipfile, only your tox
brazen charm
also seems to be creating a 3.8 venv if I'm understanding it right
how are you running flake8?
and where
patent pivot
yeah I gave you the contents of our current tox.ini
valid quest
pipenv run lint or / and flake8 in the top of the dir
valid quest
Weird
tawdry vapor
Did you sync your env?
valid quest
Yeah
valid quest
Yeah
tawdry vapor
And is ANN being used in the tox.ini rather than TYP?
tawdry vapor
If you updated from master then it should have updated the tox.ini file too
woeful thorn
It’s not weird, your environment is not in sync with master
valid quest
I fixed that, but now my commits are getting blocked even that i updated my settings
commit/patch1582222909. Flake8...................................................................Failed - hook id: flake8 - exit code: 1 Loading .env environment variables… bot/cogs/error_handler.py:32:18: TYP101 Missing type annotation for self in method bot/cogs/error_handler.py:32:28: TYP204 Missing return type annotation for special method bot/cogs/error_handler.py:36:32: TYP101 Missing type annotation for self in method [INFO] Restored changes from /home/iddos/.cache/pre-commit/patch1582222909.
woeful thorn
Your environment is still outdated
What does flake8 --version print inside the Pipenv environment
valid quest
3.7.9 (flake8-annotations: 1.2.0, flake8-bugbear: 19.8.0, flake8-docstrings: 1.5.0, pydocstyle: 5.0.2, flake8-string-format: 0.3.0, flake8-tidy-imports: 2.0.0, flake8-todo: 0.7, import-order: 0.18.1, mccabe: 0.6.1, pycodestyle: 2.5.0, pyflakes: 2.1.1) CPython 3.8.1 on Linux
Inside of pipenv
green oriole
The bot should run on 3.7 iirc
valid quest
I did sync it
Ill double check that
pipenv sync --python 3.8 --dev is that correct?
valid quest
I don't have 3.7 currently
green oriole
You should use pyenv or a similar tool to manage your python versions then ^^
You'll run into some issues for sure, I did accidentally sync it with 3.8 yesterday, didn't worked well
mellow hare
Yeah you'll for sure need 3.7 for the bot
The only project we have that's on 3.8 is snekbox
eternal owl
I ran into the same issue some time ago, I ran a git command to fix it, what was that again? @green oriole
valid quest
force?
eternal owl
Yea kinda
green oriole
Which issue? :lemonthink:
green oriole
To reset the Pipfile?
valid quest
The pip problem was a mistake uw
valid quest
Yeah, thats not it
hardy gorge
Did you update your fork from the remote? Rebased your branch? Synced your pipenv after?
The update was merged today
A few hours ago
green oriole
Basically.. git fetch origin master and git merge origin/master
valid quest
Yeah, already done
green oriole
Then pipenv --rm then pipenv sync --python 3.7 --dev
valid quest
Any suggestions?
patent pivot
errors?
stable mountainBOT
valid quest
patent pivot
okay so
that boils down to this error https://paste.seph.club/paste/90a9b55a8e.py
now what doees that mean
what command did you run to get that? @valid quest
valid quest
pipenv sync --python 3.8 --dev
patent pivot
hmmmmm
woeful thorn
What is the exact command you typed that generated the precommit error
valid quest
pipenv sync --python 3.8 --dev
patent pivot
yeah because I don't get why it would be trying to get precommit
woeful thorn
There's zero reason it should try to install precommit and not pre-commit
patent pivot
^
woeful thorn
It's never been in the lock file or in the pipfile
patent pivot
are you sure you haven't installed precommit earlier and it's been added to the pipfile or something like that?
valid quest
I will try to yeet everything, it is probably was my fault since in my mac it works fine
valid quest
If it won't work, sec, trying
Alright, i yeeted everything, it was probably my problem
@patent pivot Thanks
Now it works, i pushed the last change needed
Also i would love to hear some ideas to format it better (since bold and codeblocks aren't fitted)
Also i would love to hear some ideas to format it better (since bold and codeblocks aren't fitted)
^
hardy gorge
So you've undozzled your env? Nice.
green oriole
We are now sponsored by Sentry? Nice
patent pivot
yep yeep
green oriole
It will be used for which project(s) ?
patent pivot
all of them hopefully
site, bot, seasonalbot, snekbox
i can't think of anything else with meaningful logs
yeah that's itt
we'll roll it out project by project probably
green oriole
Nice nice
patent pivot
we've been doing a load of reworks on the devops side of things over the past week, scrags has totally changed the setup of nginx on alpha
we did deploy a logging solution as well but it was temporary
so now we're going to play with sentry
molten bough
Are they hosting it for you?
patent pivot
yep
molten bough
Lucky
green oriole
Talking about nginx, do you know if lemon have tested the localhost config type of thing, to not have to configure hostfiles for the staging bot?
patent pivot
I know there has been a discussion about it, I think we were working on SSL
I'll check now, one second
molten bough
Excellent software otherwise though
patent pivot
Okay yeah, your discussion with lemon was made into an issue on our devops repo
It'll involve local self-signed certificates but it works nicely
So we're just testing the implementation, then we'll get some guides out and update the config files
It's been confirmed to work on Linux but since it requires a little involvement to setup SSL we'll have to test on macos/windows that certificate generation/usage is smooth
green oriole
Self signed? I thought it will still trigger browser warnings
Anyway, nice!
If you need help, I have an up and running windows machine
patent pivot
You can just allow self-signed certificates
It'll warn you and you'll have to add it to your system certificate store but it will work
green oriole
Okay nice
glass pecan
@patent pivot @green oriole the setup we have in place doesn't raise a browser warning
It involves a one time generation of certs on first setting up and the certs are automatically installed as trusted on your system, ensuring the browsers accept the self signed certs without complaints.
patent pivot
ah right
rocky bloom
hey - just to let you guys know im waiting for one of my PRs in seasonalbot to be reviewed - only when you have the time though no need to rush :)
valid quest
Looks like it
clever wraith
reddit command lacking colors on embeds ;-;
marble copper
Admins u should make an ideas channel because the number 1 problem of programming is running out of ideas
brazen charm
better suited for #community-meta but there are some ideas pinned over in #python-discussion
marble copper
thx
tawdry vapor
Anyone up for some reviews? https://github.com/python-discord/bot/pulls?q=is%3Apr+is%3Aopen+label%3A"status%3A+needs+review"
green oriole
There is a P0 opened PR since a looong time
woeful thorn
Yes, we're aware of when PRs have been opened
tawdry vapor
Yeah P0 probably wasn't an accurate label in hindsight
Cause in practice the issue is quite rare
hardy gorge
It happened again a few times
In fact, it happened yesterday
I'd rather have it fixed asap still
tawdry vapor
😬 Sorry for taking so long
I should have saved the tests for later
Cause writing those took a toll on me
green oriole
Wow, Sentry is good
patent pivot
which bit did you see?
molten bough
Sentry is fantastic
green oriole
The two issues opened by the bot
Right away, you can tell that it is a good product
patent pivot
ah yeah, the issue opening is a manual thing but sentry streamlines it
the bot plugin will be really nice when a command error happens
let me try induce one
green oriole
I feel like I should learn more about it
patent pivot
!e abc
stable mountainBOT
Sorry, an unexpected error occurred. Please let us know!
ClientConnectorError: Cannot connect to host snekbox:8060 ssl:default [None]
hardy gorge
Are you breaking stuff again
patent pivot
then on sentry
brazen charm
that's a lot of info
patent pivot
there is more
it gives us the line which errored and what arguments were passed
a run up of events leading to the error
and we've got it recording the contents of commands which errored and a jump to URL
when we get site errors we get even more
it even records the SQL queries
molten bough
Traversing the stack for context for reporting is something I've implemented myself and it sure as heck wasn't as nice as this
patent pivot
we'll hook up the frontend at some point as well and be able to tell what a user clicked and stuff that led to javascript errors
we also can hook up releases as well and it will try to predict which commit introduced a bug
green oriole
Where is the pricing page haha
molten bough
It's actually open source and based on django
green oriole
Wait a second, it is an open source software?
patent pivot
molten bough
Yup
patent pivot
b1nzy went on to work for sentry
molten bough
Good place to work
patent pivot
forgot until I saw today he's now making selfbots lol
green oriole
B1nzy?
patent pivot
molten bough
He used to work for discord
clever wraith
"Please enable DMs to receive the bookmark. Once done you can retry by reacting with :pushpin:", does this line looks fine ?
do*
eternal owl
Hey @hardy gorge , should I write tests for the tags feature on the website?(PR is open)
hardy gorge
I wouldn't include it in this PR
eternal owl
ohk
cold moon
Python bot should have !style command that check pep8
patent pivot
we could run flake8 in snekbox probably
maybe wouldn't even need to sandbox it actually
so we could take links to a paste on the paste site
and return flake8 errors
brazen charm
feels like an individual command could get messy and long even on short code; but running it through something like snekbox along with the cutoff while keeping it restricted to #bot-commands sounds nice
woeful thorn
Seems like a lot more work than running it locally
patent pivot
true
brazen charm
what about an ability to run modules (python -m) through snekbox?
woeful thorn
To do what?
brazen charm
To run things like the above flake8, timeit with a more conventient interface for snippets etc.
woeful thorn
A more convenient interface?
That doesn’t sound like snekbox without a whole lot of other work
green oriole
Can you import flake8 as a module itself? There is maybe an interface that can be used through evals if we add it to the image
hardy gorge
There's currently no official stable Python API for flake8 that you can use when importing the module directly
There's a legacy API, but the docs more or less say "Don't do this until something's become available"
I'm not quite sure about adding an option to allow -m execution. It complicates snekbox and we currently have no method of storing user input as a file. In turn, that means than only things in the form of python -m module args would work.
brazen charm
@valid quest you shouldn't need to merge master unless you have conflicts afaik
valid quest
Yeah, that was wrong
valid quest
Yeah, my bad, ive seen you guys do that so i thought i need to do it also
patent pivot
did you do it from cli or through github?
valid quest
pycharm and cli
brazen charm
no need to add the additional merge commits, as it'll most probably have to be synced up at the end either way
patent pivot
because on github it assembles all needed commits into a single merge commit, through cli by default it will copy all commits over to the branch you merge to (as if you were merging from a branch to master)
valid quest
Noted 👍
hardy gorge
Yeah, that was wrong
@valid quest
Yeah, stuff happens, don't worry too much about it. I'm still a bit puzzled by what actually happened; I've never seen this effect before.
So, we're more or less speculating what was going on here
it's an interesting effect
I'm not a git master, though; just a casual user
valid quest
yeah, sorry again
brazen charm
git is still mostly a mystery to me, with pycharm making it easier to do the basic stuff I need
valid quest
How can i install the dependencies from the lock file? i did sync and install it but it won't install the sentry_sdk
hardy gorge
pipenv sync --dev should install everything; if it doesn't install sentry_sdk something's gone wrong
Which os are you on?
valid quest
yeah
hardy gorge
are you using docker to run the bot?
valid quest
yea
hardy gorge
right
you probably need to rebuild the container; is there anything valuable in your pydis database?
valid quest
I will
hardy gorge
if not, you can just do docker-compose down && docker-compose up --build
and that should rebuild the entire thing
hardy gorge
@green oriole it's a bit burried in dev-log but I finally left a review on your PR
It's nice.
One small bug
green oriole
Yes saw that thanks!
valid quest
@tawdry vapor The taglist should be sent when ever a tag is misspelt? cause i don't see when its happening
Also, do you guys know why it is still trying to commit so many commits?
tawdry vapor
Yeah cause I had to do a force push
So your local branch has a diverged history
Do you have any commits you want to save?
tawdry vapor
Otherwise you could do git fetch origin && git reset --hard origin/master to reset it to be exactly like in origin.
This will overwrite any commits you have added since then
oh sorry
That command is wrong
tawdry vapor
What was your question earlier, I didn't understand
valid quest
I dont see any case where **Current tags** embed will be called
tawdry vapor
!t
stable mountainBOT
tawdry vapor
That's when
valid quest
Oh, k
tawdry vapor
It won't happen in the error handler which is why I resolved that comment
tawdry vapor
Yes
cold moon
For SeasonalBot ImagePaginator, why I get discord.errors.HTTPException: 400 BAD REQUEST (error code: 10014): Unknown Emoji?
woeful thorn
The close reaction was changed to a custom emoji
cold moon
What should I do? I need to test .games command.
woeful thorn
Change this: https://github.com/python-discord/seasonalbot/blob/b9f010a4942c593064c2f662bb7177f3583381af/bot/constants.py#L91
Or just ignore it, if possible
cold moon
I can't ignore it, and also this don't help me
glass pecan
what's linked is the specific line in constants that you need to change to your own custom emoji that your bot can access. test bots won't see the emojis here, so you'll not be able to use them for reactions, leading to the above error. so it's a minor change to ensure you're using a test environment value when you're testing whatever you need.
valid quest
I think i already have an open Pr for this on both bot projects
I'll check, hold on
Did you guys merge them? weird
woeful thorn
Yes, that’s why there’s an issue locally
valid quest
Ok
molten bough
Requirements change, that's just software development
although I guess a tag view page would technically be possible still, but maybe better relegated to JS or something
eternal owl
they could have informed me earlier :/
molten bough
but if it's just markdown in a repo, well..
How could they? They don't have meetings until Sunday
eternal owl
or before I began working on it
glass pecan
it's impossible to inform something earlier that's only just decided
crude gyro
sorry @eternal owl, I would've informed you earlier if I could.
it wasn't decided until today
it's been discussed every which way for months
but @molten bough has a point - this is a thing that sometimes happens in software. consider it a life lesson. I've written many things I was quite happy with which never ended up being merged at all because requirements and design changed.
molten bough
yup, this kind of thing has happened in some of my projects as well
crude gyro
it sucks, but hey, hopefully you learned something that might be valuable to you further down the line.
eternal owl
okay
molten bough
I once wrote like, an entire protocol implementation for an IRC bot
and then we found a shortcoming and the entire thing had to be rewritten from the ground up before it could be released
It happens, haha
crude gyro
if you're interested, you could probably handle the issues that replaced this one - simplifying the bot tags handler and removing tags from the site. it's nice that you're writing both bot and site code for us, working full-stack as they say.
full-stack experience is very employable. 
clever wraith
hey you learned syntax highlighting now
eternal owl
I will look at those issues
crude gyro
they shouldn't be too complicated. it's mostly ripping out existing functionality and adding some fairly simple new stuff.
probably a good match for your experience level, imo.
eternal owl
okay
woeful thorn
eternal owl
Can i work on the site and bot issues simultaneously?
eternal owl
Okay I will work on both
crude gyro
@sleek mason hey, this one has been stale since late last year, do you think you might have time to address that last review soon so we can merge this? https://github.com/python-discord/bot/pull/643
sleek mason
ah shit sorry, i'm bloody awful at keeping on top of things sometimes. i'll get onto that as soon as i can. <3
crude gyro
thanks @sleek mason, you're ten million dollars baby
sleek mason
no i'm like 98 cents, but thanks!
cold moon
Huh, using IGDB API is so painful... Just almost every field have ID of resource, what mean you must do A LOT of requests.
brazen charm
working on https://github.com/python-discord/bot/issues/780, I'm not that sure where to put the definition (and the call). I started the definition in utils/__init__.py but that needs some inports that don't seem fitting along with setting up the logger and then ran it in main.py. Guessing it doesn't belong in a cog or something, where should I put it?
woeful thorn
The utils cog should be fine to start Let's make a new cog
brazen charm
Wasn't sure about the utils cog, it has a pretty ambigious description but only included commands so assumed that's what it's for
Verifier sound like a good name?
tawdry vapor
config_verifier
Maaaybe just config would work
I like short names but probably better to be more explicit
cold moon
Anyone have any suggestion for IGDB API replacement. It's too complicated API, due with games request, this return IDs of item, so you need to do extra request for every field for every game, so this response time is huge.
glass pecan
@cold moon can you describe this in more details, because i suspect there's a chance you're just accidentally forgetting the fields selector when sending a request
for example
cold moon
But watch how much reference IDs is there. Currently I had to do extra request for cover and release date, but there will be more information that I have to request individually.
Like Platform IDs, what I will add
glass pecan
pretty sure you can extend the returned data how you want with the right query in the one request
this api is pretty powerful
lets do an example
im going to simulate a search for "ori"
for the games endpoint, we have a lot of available fields
im going to fetch just the name, aggregated_rating, category, first_release_date, genres and cover
fields name, aggregated_rating, category, first_release_date, genres, cover;
search "ori";
now we have the top level fields sorted
most some of these fields are IDs for further endpoints, but by referencing their fields correctly in the initial request, we can have our response include the extra data
genre's fields are viewable over at https://api-docs.igdb.com/#genre
so we have name present in that endpoint, so we can change the query to
fields name, aggregated_rating, category, first_release_date, genres.name, cover;
search "ori";
now we have genre names in our payload
cold moon
Oh, this is amazing
glass pecan
next one is cover details
fields show the url, which is pretty much all we want
fields name, aggregated_rating, category, first_release_date, genres.name, cover.url;
search "ori";
same deal
glass pecan
as a note, you'll likely want to add commonly used enums yourself to the code
like above, games.category is an enum
if we want to filter results by only returning main games instead of all the expansions/mods/bundles/dls as well, you'll need to filter the results to only return those that have a category value of 0
fields name, aggregated_rating, category, first_release_date, genres.name, cover.url;
where category = 0;
search "ori";
like this
here's the docs on the query syntax also that is used
https://apicalypse.io/syntax/
cold moon
Should genres in Enum or fetched every time when cog load?
glass pecan
genres aren't an enum, so you can use the endpoint fields direct in queries like above instead
cold moon
Maybe is better fetch them to dict in __init__, due there will be .games genres command, what show all genres and i think there is better to save them. Also for checking
cold moon
@glass pecan Quick question: Is possible to add secondary sort for IGDB? I can't find this myself.
cold moon
Currently completed parts
Also, can anyone give suggestions about current code: https://paste.pythondiscord.com/eredaxoqap.py ?
eternal owl
You can make a PR
cold moon
This is not ready, I just hope someone give first feedback. I want to add .games company and .games company search too
molten bough
You can open a PR before you're done
it's probably better to do that actually
you can mark it as unfinished when you open the PR
in general you want to open the PR as early as possible
cold moon
Ok
brazen charm
Better chance to get some nice feedback on a pr instead of relying on the message bit being pushed back over here
clever wraith
Where is the code for Aprial fools day ?
hardy gorge
the code?
clever wraith
yea
hardy gorge
which code?
clever wraith
same like what happens in valentine's month
hardy gorge
Ah, you mean Easter
clever wraith
ahhh got it
hardy gorge
April Fool's was just a one day prank
clever wraith
cold moon
Huh, ready... Initial .games command is ready...
brazen charm
What's that latest build about?
woeful thorn
It's a test message
brazen charm
ah
clever wraith
where is seasonalbot log ?
clever wraith
Will miss you seasonalbot-log , you was the only channel that made me hype that my PR got a approvation.
brazen charm
Spotted chunks in https://github.com/python-discord/bot/blob/master/bot/utils/__init__.py when going over there yesterday thinking it could be replaced with grouper from itertools recipes, but found out it has no uses in the code beyond tests on it (that I found). Should that be removed?
brazen charm
can I throw that in inside the pr I have for verifying the channel names or a separate one?
woeful thorn
Same PR is fine
brazen charm
Remember something about removing the if not DEBUG_MODE: portion from the bot entry point, but in case it's staying putting stuff like the reddit cog there would be nice
cold moon
Can anyone review my PR (games command)?
woeful thorn
Yes, we'll get there
tawdry vapor
@brazen charm I have a PR open which gets rid of the debug check
There was a really old issue asking to remove it so I finally got around to it
woeful thorn
The reddit cog unloads itself if you don't provide API credentials
brazen charm
It's throwing exceptions for me on load
tawdry vapor
Show traceback
brazen charm
tawdry vapor
Just put some dummy values for the client id and secret
brazen charm
i justcomment out tthe load
tawdry vapor
If you'd like you could make it unload itself if it detects None
It currently only unloads itself when it fails to get an access token
woeful thorn
It already unloads itself
brazen charm
Just mildly annoying to have to uncheck the comment out when commiting
woeful thorn
What are you providing to the environment variables
brazen charm
nothing as it was configured before reddit's implementatino changed
woeful thorn
Well something is different about your environment or how it's being run because in the absence of API credentials it should unload the cog
Maybe it's a non-docker thing
brazen charm
as far as I can see it's only unloaded when it fails to fetch a token, and without the variables set the auth values default to None which fails aiohttp's BasicAuth
and that happens in the cog's init
woeful thorn
Unloading works fine when docker is used
brazen charm
Do you have anything set for the reddit values?
woeful thorn
No
brazen charm
as in removed the entries from env or their values?
tawdry vapor
I've looked at the code and you're right - it only unloads if it fails to get the access token.
Like I said, you're welcome to squeeze a change in to also unload it if there are no credentials. If not, I'd appreciate if you would open an issue for it.
brazen charm
Didn't want to do anything until I knew it just wasn't my environment. My open pr is pretty short and already is removing the function mentioned one page up so guess I can put this there too
brazen charm
For testing, don't really know anything about it but thought about looking at it for this pr (1 coro that logs when a condition is met) but saw something in the 3.8 PR related to testing. Should I wait for that or ok to start (looking at) the testing now?
hardy gorge
The 3.8 PR simplifies some things as Python 3.8 brings asyncio-support for unittest to the table.
I think we should try to get it merged soon
glass pecan
@eternal owl this is a 1:1 dump of all tags direct from the api uploaded as a private gist for you
https://gist.github.com/scragly/9923e98d0b8f497639a7ada1d21d3e21
tawdry vapor
Sad times
glass pecan
strange there's not a whitelist on that one
tawdry vapor
Which one
glass pecan
file filter
tawdry vapor
Oh I thought you were referring to an extension
Yeah, role whitelist has an issue open now
glass pecan
@eternal owl and here's a zip of the md files individually if you'd like to play with the contents locally.
the gist is mostly useful to see the current rendering differences as to what is expected
mostly of note is that simple newlines on github must have a line end in 2 spaces at the end of the line before, otherwise a newline won't be respected, resulting in running single lines for lists and similar
by doing a replace of \n with \n in order to force appending of two spaces on the line before a newline, it properly respects list formatting again:
https://gist.github.com/scragly/96b998ae03727516a85d1eff9e25a539
but this introduces extra whitespace to codeblocks if done in such an automatic manner, so it's best to just add the appropriate formatting manually once and then upload it to the bot repo as suggested in the issue ticket.
tawdry vapor
Scragly are you free for a bit? I need your expert guidance
glass pecan
i am, but i'm no expert lol. what's up
tawdry vapor
I need to resolve a race condition with on_message here https://github.com/python-discord/bot/blob/f617278a1838f53d0dc1e18e6c80f70416d2be7e/bot/cogs/help_channels.py#L277
I don't want the same channel be processed twice if messages are sent in quick succession. I was thinking of using an asyncio.Lock() for this right before it gets the available channels.
The move_to_available would need to be outside the lock so it doesn't hold it hostage while it potentially waits for the queue (granted, this would very very rarely happen)
glass pecan
you don't need to wait for move_to_available so you could schedule it as a task instead, and still just lock the entire listener behind a synchonisation primitive
tawdry vapor
Yeah I thought of that but just moving it outside the lock seems easier
glass pecan
i guess. but tbh i feel like we occasionally need synchronisation of methods in other areas also, so was thinking we could just define a sync deco utility
tawdry vapor
Definitely, but I'm not sure what that would look like. Anyway, do you think using a lock is a good approach?
glass pecan
yeah
tawdry vapor
The only downside I see is that it will also prevent handling other channels. Like if processing #help-coconut , #help-grapes can't be processed even though handling it would be perfectly fine and not be a race condition. Though in practice, the operation inside the lock won't take a long time so it's OK I suppose.
glass pecan
you could consider locking based on the channel id
i feel like i recently made a bucket class that could do this with a tweak
tawdry vapor
That's something I've wanted to have before but couldn't think of how to implement.
I'll use the lock for now. If you come up with something let me know or leave a comment on the PR, whatever works.
Thanks
glass pecan
sure. i'll have a look at this code to see if it can be reused
tawdry vapor
Oh according to my notes you have a bad memory so I'll remind you about that snekbox CI PR. It's low priority though.
glass pecan
@tawdry vapor here's a simple implementation
no locks
$ python test.py
1
2
3
3
2
1
with locks
$ python test.py
1
1
2
2
3
3
gotta add one more thing though
tawdry vapor
Thank you, I will have a look later
glass pecan
here's it with proper context manager support
and the example at the bottom has been tweaked to show how it could be used to lock based on a variable
tawdry vapor
It is basically creating a new lock for each unique key?
glass pecan
yes
but returns existing locks for known keys
like a pseudo singleton
i wanted to avoid messing with metaclasses or __new__ so i used the classmethod method
tawdry vapor
That was the naïve approach to this problem that I had. You just made a fancy interface for it. Maybe it isn't as ridiculous of a solution as I thought.
glass pecan
yep, pretty much
i wouldn't call it fancy, since it's pretty simple. it's just a hell of a lot cleaner and reusable this way, that's all
as like i said, i could see us needing this for other stuff
it's definitely not rediculous to use specific locks for each channel
tawdry vapor
I think I needed it here https://github.com/python-discord/bot/pull/755
I don't remember well
glass pecan
yeah
btw my bucket code was useless for this because it... was far more involved than i thought lol
it's also not completed, as there was a part of the api i was trying to improve. but for a simple bucket, there's 175 lines
lol
tawdry vapor
Woo the help channel cog I coded ostensibly works 🎉
glass pecan
very nice
i was thinking
what if we used a specialised CI for just linting
like, not azure, just another integration that actually provides better feedback
woeful thorn
Better how?
glass pecan
for example, if an integration could better support the github api, we'd be able to have linting issues added as a comment on the actual relevant code line
potentially, it could even support a "fix it" type thing for simple fixes, like as a suggested change.
woeful thorn
bUT YoU ShOULD LiNt befORe yoU Push
glass pecan
lol
woeful thorn
Are there services that offer this already? Suggesting replacements seems complicated
glass pecan
the point is mostly that in it's current form, azure isn't making the most of the github api
and i do know theres things that exist like it, i don't know if it's existing for python specifically though and i haven't yet checked
i'm just spitballing atm
woeful thorn
Or are linting errors at least better exposed with a GH action?
glass pecan
GH action has a better interface to make use of the API, sure
woeful thorn
But do they
glass pecan
that's up to the tasks
if we use a pre-made task, we'd have to check with it
if we made our task, we'd have to write those stages in
i doubt though any tasks/recipes for actions would have some fancy suggested changes feature though. at least not yet
woeful thorn
Something like this? https://github.com/grantmcconnaughey/lintly-flake8-github-action/pull/1#pullrequestreview-338419294
glass pecan
mmm no
that's not very good feedback
lol why'd you pick the one comment to link that didn't have the line comments
further down they have line comments, which is a hell of a lot more useful
no suggested changes involved ofc here, but that's a fancy extra that's unnecessary
sure would be tasty to have tho
woeful thorn
It’s the link from their read me
glass pecan
then again, if we went with black...
woeful thorn
Good luck lol