Try docker-compose up bot to start up just the bot and attach to it
#dev-contrib
gritty wind
wraith kayak
Error:
raise ClientConnectorError(req.connection_key, exc) from exc
bot_1 | aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host api.pythondiscord.local:8000 ssl:default [None]
Ah okay
I think I remember the solution for this
Do you have changes in your config?
wraith kayak
Like what all changes?
wraith kayak
urls:
# PyDis site vars
site: &DOMAIN "pythondiscord.local:8000"
site_api: &API !JOIN ["api.", *DOMAIN]
site_paste: &PASTE !JOIN ["paste.", *DOMAIN]
site_staff: &STAFF !JOIN ["staff.", *DOMAIN]
site_schema: &SCHEMA "http://"
site_logs_view: !JOIN [*SCHEMA, *STAFF, "/bot/logs"]
paste_service: !JOIN [*SCHEMA, *PASTE, "/{key}"]
Right, so set site to web:8000
https://pythondiscord.com/pages/contributing/bot/#configure-the-bot
And I think it should be able to start now
wraith kayak
Yay! It's online now 🥳
Now I can make changes to the bot for the purpose making contributions right?
gritty wind
Yeah, you should be good to go
Check the contributing guide on the repo to get you started
wraith kayak
Thank you so much for guiding me and fixing my errors!
gritty wind
Lol docker can be a bit of a pain, but this should only be a one time setup hopefully
wraith kayak
Hopefully yes 🙂
spiral orchid
@gritty wind I've a doubt, like the pagination has been implemented but the response has been changed, so do I need to update the response or the tests?
hollow vine
After I say pipenv run precommit I get this error below. How can I fix this?
C:\Users\Ş. Evren Alhas\bot>pipenv run precommit Exception in thread Thread-4: Traceback (most recent call last): File "c:\anaconda3\lib\threading.py", line 926, in _bootstrap_inner self.run() File "c:\anaconda3\lib\threading.py", line 870, in run self._target(*self._args, **self._kwargs) File "c:\anaconda3\lib\subprocess.py", line 1267, in _readerthread buffer.append(fh.read()) File "c:\anaconda3\lib\encodings\cp1254.py", line 23, in decode return codecs.charmap_decode(input,self.errors,decoding_table)[0] UnicodeDecodeError: 'charmap' codec can't decode byte 0x9e in position 9: character maps to <undefined>
Traceback (most recent call last): File "c:\anaconda3\lib\runpy.py", line 193, in _run_module_as_main "__main__", mod_spec) File "c:\anaconda3\lib\runpy.py", line 85, in _run_code exec(code, run_globals) File "C:\Anaconda3\Scripts\pipenv.exe\__main__.py", line 7, in <module> File "c:\anaconda3\lib\site-packages\pipenv\vendor\click\core.py", line 829, in __call__ return self.main(*args, **kwargs) File "c:\anaconda3\lib\site-packages\pipenv\vendor\click\core.py", line 782, in main rv = self.invoke(ctx) File "c:\anaconda3\lib\site-packages\pipenv\vendor\click\core.py", line 1259, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) File "c:\anaconda3\lib\site-packages\pipenv\vendor\click\core.py", line 1066, in invoke return ctx.invoke(self.callback, **ctx.params)
File "c:\anaconda3\lib\site-packages\pipenv\vendor\delegator.py", line 248, in block stdout, stderr = self.subprocess.communicate() File "c:\anaconda3\lib\subprocess.py", line 964, in communicate stdout, stderr = self._communicate(input, endtime, timeout) File "c:\anaconda3\lib\subprocess.py", line 1317, in _communicate stdout = stdout[0] IndexError: list index out of range
tag me when you reply please
short snow
bump.
short snow
and about this?
another feature for bot, just like tldr pages, we can add this or the tldr one.
cold island
and about this?
Can you link the line in the code?
cold island
@short snow it does work, findall just behaves differently
#bot-commands message
It only showed you the groups
short snow
ok thankyou!
short snow
@vocal wolf @green orioleMentioned all the requested changes!
spiral orchid
<@!386599446571384843> I've a doubt, like the pagination has been implemented bu...
Anyone ?
gritty wind
``` File "c:\anaconda3\lib\site-packages\pipenv\vendor\delegator.py", line 248, ...
I can't be fully sure, but seeing that you're getting an error in your dependencies, it might be due to running in anaconda. Try to recreate the pipenv from your base python install
gritty wind
<@!386599446571384843> I've a doubt, like the pagination has been implemented bu...
I haven't looked at your PR, but if the new response can still be used by our other apps such as the bot, update the tests. If not, you have to convert it to the format expected.
spiral orchid
the initial response was an array but after pagination, the response is a dictionary and the result will be present under the results.
gritty wind
In this case, you definitely update the response, not the tests
spiral orchid
and if you want the next infraction page you need to pass the limit and offset also
spiral orchid
In this case, you definitely update the response, not the tests
Okay, I'll update
gritty wind
It could be possible to have the bot automatically fetch the next batch when it runs out, but that would require more changes on the bot & site
cc @green oriole @cold island
if we were to do that, we might also be able to bring the limit back down to 50
gritty wind
It could be possible to have the bot automatically fetch the next batch when it ...
(This isn't something you need to do in your PR, at least not yet)
spiral orchid
This is the response after pagination
spiral orchid
(This isn't something you need to do in your PR, at least not yet)
So the left part is to update the response as previous.
gritty wind
Yeah
spiral orchid
thank you
green oriole
It could be possible to have the bot automatically fetch the next batch when it ...
Well, we would run into the same issue where the bot would try to retrieve 10k infractions and it would take ages
gritty wind
We use the paginator and fetch as needed
We only fetch 50 at a time, and move the offset if we need to fetch more
Or hell, we can probably simplify it by fetching enough for one page, and use the page number as offset
Though that would be way more requests than 50 at a time
green oriole
Hmm I'm not sure we will ever read more than 100 infractions to be honest haha
eternal owl
how does self.bot.wait_until_guild_available() work? I am having an issue where the program is stuck on this line, i.e, guild is not yet ready/available for a long time
green oriole
Are you using the staff test server?
green oriole
Hmm, I believe you need to configure the guild ID in your env file
eternal owl
oh let me check that
yep, you are right
I remember changing servers
thanks a lot for your help !
gritty wind
If that doesn't work, could the cache just not be available
brazen charm
May be a good idea to log instead of just returning early when the ids don't match
eternal owl
works now
hollow vine
I can't be fully sure, but seeing that you're getting an error in your dependenc...
Do you mean new env?
gritty wind
Yeah, without using anaconda
green oriole
May be a good idea to log instead of just returning early when the ids don't mat...
Hmm.. Like having a timeout?
brazen charm
Hmm.. Like having a timeout?
I was thinking of just doing a trace log in here https://github.com/python-discord/bot/blob/3d014d95f57633a2f8fdf40f61ed1b0ade7343dc/bot/bot.py#L239-L240 but logging the reason with a higher level if it isn't available after a set amount of time might be clearer
green oriole
Sounds good to me
gritty wind
Yes
The docker compose sets up the site, as well as a few other pydis projects
But really you only need the web and postgres
short snow
so i just do docker compose-up after cloning the site?
and then i can work on the bot with (pipenv run start)
gritty wind
Yup, or you can use the docker compose of the bot
I personally use the site since I already have it cloned and can make modifications
green oriole
If you don’t want to develop the site, you can just use the compose of the bot
short snow
ok, i will setup the site with docker, and use pipenv to work on the bot. Ty!
cold island
for most purposes you can use docker for both
short snow
docker takes time, so for development purposes i just use the normal way, and for others use docker
cold island
docker takes time
Not sure what you mean by that 🤔
short snow
hmm, like when u do docker-compose up, it takes ~10 min
gritty wind
It should only do that when building all the containers
Once built, you can rebuild only the bot one and be up in no time
short snow
so i do docker-compose up once again, it will be up in no time?
gritty wind
You can try it now 👀
short snow
ok, once school gets over, will do that
brazen charm
It shouldn't be very slow, but if you already have it set up with pipenv there's not much benefit from moving the bot to docker
gritty wind
I personally run the bot without docker, so I can get debugging with my IDE, but as long as the bot runs, do whatever you like
short snow
i will keep the bot on pipenv, but will do site on docker (would make it easier)
brazen charm
yeah site postgres and redis are easier with docker as you don't need to install all the services locally
short snow
another feature for bot, just like tldr pages, we can add this or the tldr one.
i will just open a issue for this, wld be better
thanks for your time
brazen charm
Issues are generally better if you need discussion around a feature
late wolf
Why did the bot send this message
#python-discussion message
out of no where?
gritty wind
Presumably someone ran a command and deleted their message
green oriole
Can confirm
late wolf
right
short snow
in one issue am assigned but its status is planning, so i should wait right?
short snow
.issue 568
dusky shoreBOT
gritty wind
Don't work on it yet, do you know why it is in planning@vocal wolf
Ah stupid question, it just wasn't approved
Can you approve it when you see this
vocal wolf
Can you approve it when you see this
done
cold island
the former
green oriole
You can just make an error embed
short snow
but it will return a 404-not-found image, and there is no way to check if that matches with what the user gave
short snow
>>> import requests
>>> r = requests.get("https://httpstatusdogs.com/111")
>>> r.json
<bound method Response.json of <Response [200]>>
Hmmm... that’s a bit silly
brazen charm
Can just check for the redirect
short snow
how can we check that?
green oriole
Use allow_redirect=False, or check the length of the Response.history attribute
brazen charm
It'll need a special case for the 404 return code but should be much simpler than querying the site for what they have available
short snow
yeah
short snow
should i use codes from the standard lib, or use regex+aiohttp to get them?
that's what i meant here in the second option
brazen charm
Where does regex come into play here? Validate the status code with http, do the request, if 404 was not requested check the redirect and send an error if it redirected to the 404 page
short snow
async with self.bot.http_session.get('https://httpstatusdogs.com/') as response:
text = await response.text()
http_codes_dog = re.findall(r'<a href=\"(\d{3})-[^\"]*\"', text)
Parsing html with regex 
short snow
the only solution i cld think of
late wolf
did you forget bs4?
short snow
Parsing html with regex <:lemon_grimace:754601078561046558>
should i use that?
late wolf
Why not use bs4?
gritty wind
Can you find a restful api I like the other solution waaay more
green oriole
I’d prefer to check if you have been redirected to the page
short snow
i don't like bs4 for this, mine is probably a better way than bs4
gritty wind
Where does regex come into play here? Validate the status code with http, do the...
what about this?
short snow
that's what akarys says basically, but isn't that the more longer way.
i don't understand, why shouldn't the parsing html with regex solution be used?
green oriole
It is way less error prone and probably easier to implement
gritty wind
Someone wanna link the html parsing copy pasta?
green oriole
If the html changes, the code breaks
timid sentinel
If you look at the website they just store the images like this, why not just try and fetch it directly? https://httpstatusdogs.com/img/503.jpg
green oriole
You cannot browse this page at “stackoverflow.com” because it is restricted.
green oriole
If you look at the website they just store the images like this, why not just tr...
That’s the idea, but they will redirect you to the 404 image for non-existing codes
green oriole
Depends on your accept header, I’m pretty sure
brazen charm
Parsing the html with regex wouldn't be a big problem as it's a small subset, but you're relying on the html being static and it'd need to be updated to be up to date in case of changes and the additional complexity around that imo outweighs the benefits of preventing occasional redundant requests
green oriole
Checking the length of history is very easy though, why don’ we go for that?
short snow
```py
async with self.bot.http_session.get('https://httpstatusdogs.com/') as res...
@gritty wind
gritty wind
Why parse the html though
I'm making no-header get requests to the img url, and getting things back as expected
It either returns a page with a jpg and a content-type header, or it returns the 404 page
import requests
codes = ["400", "200", "602", "404"]
for code in codes:
page = requests.get(f"https://httpstatusdogs.com/img/{code}.jpg", allow_redirects=False)
print(page.url)
print("Image Found" if page.headers["content-type"] == "image/jpeg" else "404ed")
Actually went ahead and wrote it pretty much
import requests
import shutil
codes = ["400", "200", "602", "404"]
for code in codes:
page = requests.get(f"https://httpstatusdogs.com/img/{code}.jpg", stream=True)
if page.headers["content-type"] == "image/jpeg":
print(f"Saved Code: {code}")
with open(f"codes/{code}.png", "wb+") as file:
shutil.copyfileobj(page.raw, file)
else:
print(f"404 On: {code}")```hmm, basically parsing the html with regex does that in a much shorter way.
green oriole
I don't think we should rely on this, it sounds pretty much like an implementations detail
sharp timber
Why not check page.status_code
wintry trellis
Hello!
Under Test Server and Bot Account in Sir-lancebot contributing guide, it says
4. Create the following roles:
@Admin
and under Environment variables, it seems ID of role Helpers is needed. So shoudn't @Helpers need to be added in Create the following roles?
Source: https://pythondiscord.com/pages/contributing/sir-lancebot/#working-with-git
cold island
Why not check `page.status_code`
200 even when the page doesn't exist. Returns the 404 image by default
sharp timber
O.o
cold island
although that might not be the case if redirects aren't allowed? not sure I followed that far
gritty wind
Not allowing redirects didn't seem to do anything
gritty wind
Hello!
Under `Test Server and Bot Account` in Sir-lancebot contributing guide, i...
If you are following the guide, you should have created your server using the template
Which creates all the roles and channels
If you don't use the template, you need to create it, yes
sharp timber
I get a 302 redirect when I hit the wrong image
>>> requests.get(f"https://httpstatusdogs.com/img/3.jpg", allow_redirects=False)
<Response [302]>
>>> requests.get(f"https://httpstatusdogs.com/img/302.jpg", allow_redirects=False)
<Response [200]>
We can also check response.url for the final url
and if it matches their 404 page we can discard it, perhaps
>>> requests.get(f"https://httpstatusdogs.com/img/3.jpg", allow_redirects=True)
<Response [200]>
>>> _.url
'https://httpstatusdogs.com/404-not-found'
I still don't understand why you would do that when you can check the return type :P
The redirect page, and the 404 page are text, the correct ones are images
Am I missing something
cold island
the status code sounds more reliable
wintry trellis
If you are following the guide, you should have created your server using the te...
Yes but "creating a server using template" is mentioned in https://pythondiscord.com/pages/contributing/bot/ and not in Sir-lancebot contributing guide. So if it's added in that guide too, it will be helpful for newcomers :)
sharp timber
I don't like relying on content-type as it's not something as nailed down as the status code or final url
They could, for example, start serving webp images, it's happened with other sites to me before
It is a good test check though, to make sure things are as expected
short snow
Actually went ahead and wrote it pretty much
```py
import requests
import shutil...
but for this we need the list of codes
gritty wind
Not really, you can replace codes with the user input
But yeah the status code might be a better bet
short snow
so i should use aiohttp to get the result rather than just putting the code in the link
and check for the response?
gritty wind
```py
>>> requests.get(f"https://httpstatusdogs.com/img/3.jpg", allow_redirects=...
^ this pretty much. Replace 3 with the user input, and you'd get what you need
If the response is 200, you got an image. If it is a 302 you got a 404
short snow
so i should use aiohttp to get the result rather than just putting the code in t...
so basically this
sharp timber
Probably should filter it to numeric and <= 4 digits for safety as well
brazen charm
It can pass through the same check as the cat command
sharp timber
to bot-commands to play with the cat command
brazen charm
it checks the codes against the http module's list of codes, here it needs the another check but still prevents the erroneous inputs
short snow
#sir-lancebot-playground message why was no error embed sent in this?
should i add another check for it to be less than 4 digits ( == 3 digits)
subtle kraken
https://discord.com/channels/267624335836053506/607247579608121354/8039574489468...
exceptions that aren't handled on @dusky shore aren't delivered to the user
short snow
oh, so that is only sent by bot
should i add that check to http_cat then?
while am doing http_dog?
subtle kraken
the issue as to why it didn't send was due to the fact that embed title can only hold up to 256 chars, more than that will make discord return an exception
but sure I suppose
short snow
ok.
trim cradle
Isn't there some other caveat about the config file intended to solve aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host api.localhost:8000 ssl:default [None]?
oh it might be working now
gritty wind
I mean what is there to solve?
trim cradle
I mean what is there to solve?
My bot instance isn't online.
Now I have
web_1 | January 27, 2021 - 17:31:29
web_1 | Django version 3.0.11, using settings 'pydis_site.settings'
web_1 | Starting development server at http://0.0.0.0:8000/
web_1 | Quit the server with CONTROL-C.```
which I think is what is expected, but the bot isn't online.I'm running this on linux now btw.
trim cradle
!paste
brazen charm
is everything in docker?
trim cradle
trim cradle
is everything in docker?
I assume so.
gritty wind
You'd need to change the url in the config
trim cradle
You'd need to change the url in the config
to what
gritty wind
web:8000 iirc
green oriole
Yeeep
trim cradle
why isn't the config file on notion already like that, then?
green oriole
Yes
green oriole
is docker only for if you're on windows or something?
Nah, you can use docker everywhere
gritty wind
If you're running on docker you'd use web:8000 (because you didn't do hosts modification on your docker). If you are running on localhost, the site is mapped to pythondiscord.local (assuming you followed the guide)
green oriole
Actually, localhost:8000 will work now
trim cradle
I'm also not clear on which host I'm updating
green oriole
pythondiscord.local isn't mandatory now
gritty wind
Your system host, so it can "bind" pythondiscord.local to the site url so to speak
trim cradle
Your system host, so it can "bind" `pythondiscord.local` to the site url so to s...
"system" doesn't appear in the config file.
gritty wind
System as in your OS system
but apparently you don't need to do that anymore, which is way easier usually
brazen charm
If it's accepting all hosts we should just get rid of the hosts file part and point at either localhost or host.docker.internal depending on how it's set up
gritty wind
I think linux hosts should be under /etc/hosts if you do want to go down that route
brazen charm
(if it isn't going through docker's names)
green oriole
@trim cradle if you use docker, just replace localhost by web in the config file
trim cradle
<@253696366952316929> if you use docker, just replace `localhost` by `web` in th...
all three instances of the substring "localhost"?
gritty wind
The other ones aren't mandatory for running the bot
Only the site is
So configure them if you want their features
green oriole
Only the second one
gritty wind
(Redis can use fakeredis)
green oriole
The first one should be redis and the last one should be snekbox
trim cradle
So configure them if you want their features
so change line 319 to read exactly site: &DOMAIN "web:8000" with no other changes?
gritty wind
Yesh
green oriole
Yep
trim cradle
Alright, now I'm having a token-related error, so I'll regenerate those, I guess.
gritty wind
Why do we do all this btw? Can't we just make it one url, and bind the same ports on host and docker
That way it can all be localhost:8000
trim cradle
Woo it's online! Thanks @gritty wind @green oriole @brazen charm
green oriole
Niice
green oriole
Why do we do all this btw? Can't we just make it one url, and bind the same port...
Well, not really because of how the virtual Docker network works
I'll try to work out an easier method
gritty wind
Well, not really because of how the virtual Docker network works
Not if we use --net host 👀
I actually had to do that for metricity because
reasons
spiral orchid
@gritty wind code has been pushed. Review the changes and let me know if something else needs to be changed.
gritty wind
You still have a problem with your imports starting on line 6
https://github.com/python-discord/site/pull/443/files#diff-9d0308e109cd6c4edf9d58d1c054daa10f3f84dd74ae4d2572adc9e194f726a3R6
If you are importing things over multiple lines like that, our code style would have you put each entry on a separate line
Like it was before your changes
You can see the full style guide here https://pythondiscord.com/pages/contributing/style-guide/
Same thing applies to line 13
This also seems to be an erroneous new line https://github.com/python-discord/site/pull/443/files#diff-215b87c752e5a7b58b8ec5d58accbdcf4ab3bd325a171301dd67c6ae4852f2b8R234
Someone more familiar with django will have to do the actual functionality review
spiral orchid
thanks
short snow
@gritty wind can you have a look at c4 when you are free 
gritty wind
I had a look over it this morning, expect a review soon. I don't think I'll be requesting any more major adjustments, just a few minor notes here and there
That will have to come tomorrow though, it's kinda 2 in the morning here
short snow
#sir-lancebot-playground message no bot reply here, looks like it has caught an exception
gritty wind
Issues are turned off for that repo
> GET https://api.github.com/repos/torvalds/linux/issues/1
{
"message": "Issues are disabled for this repo",
"documentation_url": "https://docs.github.com/v3/issues/"
}
short snow
ah ok
short snow
Am loving reviewing the markov poem problem, and learning markov.
late wolf
So in our bot/utils/extensions.py (suppose this file is A)
we have this line
imported = importlib.import_module(module.name)
And we are importing this file in our bot/exts/utils/extensions.py. (suppose this file is B).
Here A is importing B and B is importing A, but how do we avoid importerror?
green oriole
I usually use a relative import in one of the two
I'm honestly not sure why that does break the circle, but hey
late wolf
Huh?
My question was like it doesn't error out, how do we actually avoid that. not how to avoid it
sorry if i am not making sense
green oriole
Ah, well, no idea haha
late wolf
Huh?
gritty wind
Maybe even in the init if they are in the same module
So like file A and B both import from file C
No circular import
late wolf
let me check if they both import from a common file
gritty wind
I’m saying they can if you need to share variables across both
late wolf
I think you are misunderstanding me, should I rephrase my question again?
gritty wind
Yeah sorry. Wasn’t the problem with a circular import?
late wolf
So in our bot/utils/extensions.py (Lets consider this file as A) we import all the modules from bot/exts to check if they have the setup function.
And, in our bot/exts/utils/extensions.py (Lets consider this file as B) we import bot/utils/extensions.py.
So file A is importing file B through importlib.import_module and file B is importing file A.
Because both of these files are importing each other this is supposed cause an ImportError because of circular import, but how come this doesn't cause an import error?
Was my question
gritty wind
Ah okay
This is the ext loader, right?
It doesn’t load any files that don’t have a setup method
late wolf
But it still imports them
late wolf
aight
stable mountainBOT
sharp timber
There's no circular import because importlib.import_module has a loop-breaking construction: if it's current importing a module that then is imported, it will instead return a reference to the currently-partially-loaded module
from imports don't permit this, and thus will error if there's a loop, but if there aren't from imports then python happily lets you do import a import b in two files. Keep in mind that if one of these is the main script, you can load your main script as the main module and the __main__ module (given it's named main.py), which is twice, compared to other modules, which will load once
late wolf
Ah got it, so importlib.import_module doesn't actually import the module but give a refrence to it.
Thankyou
sharp timber
Right, if the module is already imported
late wolf
Wdym by that
sharp timber
# pseudocode
def import_module(name):
if name not in sys.modules:
sys.modules[name] = empty_module()
actually_run_code(name, in=sys.modules[name])
return sys.modules[name]
Or, well, (edited)
late wolf
😕
sharp timber
I missed a step, which si that the empty module is created before running the code
So, basically, at the very top of your code sys.modules["my module name"] exists
and thus, import my_module_name simply grabs that object that's there
late wolf
Well this (<#dev-contrib message>) was all I needed to know, thx
stable mountainBOT
@gritty wind
It has arrived!
gritty wind
Good bot, go back to your hole
stable mountainBOT
vale ibex
Any ideas about bot_1 | aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host api.localhost:8000 ssl:default [None] from bot in docker?
This looks like the relevant parts of the traceback bot_1 | File "/bot/bot/bot.py", line 79, in cache_filter_list_data bot_1 | full_cache = await self.api_client.get('bot/filter-lists') bot_1 | File "/bot/bot/api.py", line 80, in get bot_1 | return await self.request("GET", endpoint, raise_for_status=raise_for_status, **kwargs) bot_1 | File "/bot/bot/api.py", line 74, in request bot_1 | async with self.session.request(method.upper(), self._url_for(endpoint), **kwargs) as resp:
I'm guessing this is something to do with the site?
gritty wind
Sort of
The site is probably up
but the docker url would be web:8000
not localhost:8000
vale ibex
Ahh that's sorted it, I used the config from notion
gritty wind
That one assumes you're running the bot locally
vale ibex
makes sense
gritty wind
Maybe I'll just add a note now
vale ibex
it's in the contributing guide notes If running the webserver in Docker, set it to "web:8000"
I just didn't pay attention it seems 😄
gritty wind
Lol dw a lot of people run into similar problems
I've written it down now on the config
vale ibex
.issue 1375 bot
dusky shoreBOT
vale ibex
Would ArgumentParsingError be the best error to raise for this?
Since BadArgument sends the help embed too, which I don't think we want
or do we?
Hey, if you need help check out #❓｜how-to-get-help . This channel is for discussions around the PyDis repositories.
gritty wind
I think we would want to send the help embed, because that's the default behavior for other conversion errors
for instance:
!remind u12ih3uh
vale ibex
makes sense, then I think I've already got a fix 😛
inland hill
vale ibex
pre-commit seems to be trying to force LF line endings, whereas all the others are CRLF
Should they be LF?
vale ibex
Ahh, must have been my global conf that force it on clone then
git config --global core.autocrlf true 
gritty wind
It'll revert the change to LF
but you can submit it with the files changed, and it won't show up by the time it gets to the repo
What I did when I was fixing it on mine was I pushed all my code
Copied the git folder
deleted, and recloned the repo with the updated config, and put the git folder back
vale ibex
yea, probably be the easiest. I'll do that too
clever wraith
kl
short snow
#sir-lancebot-playground message on game over, clearing the reactions would be a good idea
short snow
@gritty bolt check out your pr! Gave a review
short snow
i can delete .reddit msgs of others
vale ibex
Looks like another issue to be added to https://github.com/python-discord/sir-lancebot/issues/348
gritty wind
from bot import constants
from bot.constants import MODERATION_ROLES
hungh
oh i broke policy bot
what was the fix, close and reopen?
gritty wind
Though it is still errored
Error evaluating policy defined by python-discord/bot ref=master
patent pivot
refresh?
gritty wind
Alright, just gihub being slow then
patent pivot
thanks golang
gritty wind
Did it just put the entire program into the traceback
patent pivot
lmfao
it's just a long stack trace
so it seems to be a bug that sometimes GH API doesn't return the pushed date immediately after the push
so our infra is TOO FAST
gritty wind
Is there any pattern to the commits that can cause it?
This one seems to have been merging master into the branch
patent pivot
yeah I think I've seen it on a few master merges
green oriole
Damn, we are too fast
patent pivot
ak please compliment my infra
patent pivot
thank you
green oriole
Can we make it less good though?
thorny obsidian
:3
gritty wind
Actually I can do it
green oriole
Yeah
gritty wind
Yesh
green oriole
!help
stable mountainBOT
Command Help
gritty wind
In that case, can an admin try and delete mine
gritty wind
2 instances reported of the bot still allowing people to delete other's embeds in #helpers. Can anyone repro locally?
subtle kraken
@gritty wind do you recall which file had functionality for that?
gritty wind
One sec
tawdry vapor
Paginated ones or normal?
subtle kraken
the one that does not work as intended
gritty wind
one tag, one doc, and possibly one help
tawdry vapor
That's what I'm trying to figure out - which one doesn't work
subtle kraken
either can work then
vocal wolf
https://github.com/python-discord/bot/pull/1388 a review is needed for sanity reasons
tawdry vapor
Does the command error handler not handle exceptions raised by converters?
That error handler has a special case for errors from our API already
gritty wind
There is no converter here?
tawdry vapor
What? It's literally in converters.py and subclassing Converter.
vocal wolf
Does the command error handler not handle exceptions raised by converters?
I guess not, since the error raised by sentry https://github.com/python-discord/bot/issues/1375
Would've be handled by https://github.com/python-discord/bot/blob/master/bot/exts/backend/error_handler.py#L78
tawdry vapor
Cause it's wrapped in a ConversionError instead of a CommandInvokeError.
vocal wolf
Ah
tawdry vapor
I think the better approach is to check the inner exception of the ConversionError too.
vocal wolf
I think the better approach is to check the inner exception of the `ConversionEr...
@vale ibex you probably want to add this to your current PR.
tawdry vapor
The current fix is too simplistic anyway, and it doesn't cover the other request done in the converter
It'd be nicer to take advantage of all of this
vale ibex
Actually looking at it, the current approach has something weird, where 3-4 minutes after the error is raised, it then sends the debug message bot.exts.backend.error_handler | DEBUG | Command infraction edit invoked by Chrisjl#2021 with error BadArgument: The provided infraction could not be found.
tawdry vapor
vale ibex
Actually looking at it, the current approach has something weird, where 3-4 minu...
Ah, this seems to be a delay for other errors too, not just this 1
vale ibex
Updated the PR
vale ibex
.issue 1365 bot
dusky shoreBOT
vale ibex
Been working on this, it seems like the current impl errors if you give an ID of a verified bot. (575252669443211264) is an example ID of one.
It just needs a new emoji to be added for :badge_verified_bot:
What's the process for that?
gritty wind
If you need to add something to the config, add it, and possibly leave a message on the PR alerting people they need to add it to their configs
(also would be awesome if you update the test server config once its merged)
tawdry vapor
I think the question is how does the emoji actually get added
The answer is that you need to ask a staff member to do it, providing the emoji image if you have one
gritty wind
Isn't the verified sticker a default discord one on bot accounts?
tawdry vapor
Not all bots are verified
vale ibex
Yea, verified bots have this
Annoyingly its not an image I can just pull off discord, so I'm currently trying to find an image suitable for an emoji
tawdry vapor
It's just css + html
You could copy it over to a blank page and let the browser scale it up
vale ibex
the tick is an svg too 😛
tawdry vapor
Or maybe just zoom in
vale ibex
True, i'll give that a go
tawdry vapor
I could create the whole thing in SVG to be "proper" but we don't really need an SVG for this
cold island
Been working on this, it seems like the current impl errors if you give an ID of...
Seems like Akarys is assigned to this. He might not mind, but for future reference you should assign yourself to issues before working on them to prevent clashes and two people doing the same work
cold island
no worries. As a helper you can assign yourself to issues without asking, just make sure it's not already taken
vale ibex
I'll leave it for now then, and just put my findings in the issue 😄
short snow
@kind flower i had to change it to or, since the
It has to be either in the categories
or
in the organisation
it can't be in both, so and won't fit
or would be perfect
kind flower
but you are checking for when to not run it right? @short snow
or did I read the code wrong
short snow
if the message is in the whitelisted categories
or
if the message is in the whitelisted channels (i.e. only organisation in case of on_message)
then, u continue
else we don't run it
yes, after your latest commit it is correct, but it was not at the time of me leaving the review
kind flower
also while I have you here, what I meant with PYTHON_DISCORD_REPOS is that the url does not mention pydis specifically, so personally I would either drop the formatting and just put it in the url from the start, or change the variable name to something more generic
short snow
PYTHON_DISCORD_REPOS = "https://api.github.com/orgs/{repo}/repos"
it should be
REPOS_API = "https://api.github.com/orgs/{org}/repos"
kind flower
personally I think that is better
short snow
async with self.bot.http_session.get(PYTHON_DISCORD_REPOS.format(org="python-discord")) as resp:```and this
kind flower
can't type today
short snow
Will do that!
short snow
for on_message, it is in development, media, devprojects (Categories) or organisation channel
and for issue:
kind flower
the original issue (#566) mentioned it should be limited to:
the organisation channel (551789653284356126)
the development category (411199786025484308)
the dev-projects category (787641585624940544)
the media category (799054581991997460)
short snow
it is in development, media, devprojects (Categories) or white_lsited_channels
well gtg. will do those changes, if anything else is there, just put it on the pr
short snow
@cold island the #Mod-tools and #mod-meta are channels right?
subtle kraken
yes they are
green oriole
We could whitelisted the whole category tbh (CC: @cold island)
short snow
(ping me when u decide what to whitelist.)
@gritty wind can we shift issues from one repo o another, do i need to close one issue and open on the sir-lancebot
gritty wind
No we can transfer it, if no one has any more concerns
short snow
ok, you can do that then ig.
gritty wind
donezo
short snow
ah that was fast
gritty wind
Yeah lol it is pretty much one button click on the github ui
short snow
nice, didn't know that
gritty wind
It's the first time I use it myself, pretty cool
gritty wind
Damn it, now I'll have to update my metricity container smh
patent pivot
lul
nothing much has changed
@short snow hey
can you test something
(cc @sharp timber)
can any regular member try hit the trashcan on this
!paste
late wolf
it didn't do anything
patent pivot
cheers
sharp timber
mine worked but yeah
gritty wind
I've done plenty of testing on server, and off server, but even under the exact same conditions, I could not get it to replicate
sharp timber
#help-grapes message is the invocation that cancelled out
patent pivot
do we have any logs for this
sharp timber
Got a link to the source?
gritty wind
(Someone tried to use a tag on cooldown which is the 1 log)
You're either looking at the pagination source, or the docs/help/tag source
Pagination source:
https://github.com/python-discord/bot/blob/master/bot/pagination.py#L188
sharp timber
ty
gritty wind
And the tag invocation:
https://github.com/python-discord/bot/blob/master/bot/exts/info/tags.py#L268
sharp timber
Yeah, I can't find anything
gritty wind
The code is solid, and I can't repeat it on my smaller scale test server
So i'm thinking it's some oddity with discord, or the library
but I can't confirm
green oriole
Is the or and not working as we would expect it to?
gritty wind
I tested, it is
sharp timber
Are we fucking up how ctx.author behaves?
green oriole
Like, if we get a Userobject, will it always return true?
gritty wind
I also checked the mod roles on the deploy, and they work
gritty wind
Like, if we get a `User`object, will it always return true?
I tested that specifically, and no it works as intended
That is to say, the check fails
patent pivot
hahahaha this is a conundrum
gritty wind
Are we fucking up how `ctx.author` behaves?
But why would it only fail sometimes 🤔
green oriole
Hmm
sharp timber
Maybe something edits ctx.author
eager fable
Hello :)
gritty wind
Hmm, let me get you a full invocation trace
green oriole
Are we passing a mock author at some point, or something
Or are we just completely overlooking the issue?
We probably are tbh
gritty wind
Can we add some logging in there
if we log warnings for all invocations, we can check on sentry
green oriole
hahahaha this is a conundrum
Thank you Joe, I learned a new word today
patent pivot
green oriole
No no, let's not use warnings
gritty wind
Does sentry log anything lower
late wolf
Thank you Joe, I learned a new word today
Me too
green oriole
Is there a way to reduce the grafana log level to trace?
patent pivot
nah
green oriole
Sad
gritty wind
There is nothing being logged anyways
patent pivot
grafana just pulls out what is coming out of the pod
and the pod silently drops all debug & trace in prod
gritty wind
I wanted sentry specifically because it automatically gets all the defined vars
green oriole
Hmm.. if we add an env var to configure this, can you temporarily force trace in prod?
Using sentry is pretty hacky
gritty wind
That would be a lot of logging while we wait for the error to happen again
it isn't reliably reproducible
we could middleground it and log info
sharp timber
Maybe some strange race condition?
subtle kraken
I do wonder what shenanigans are we doing that paginator doesn't error out
gritty wind
cooldown, try #bot-commands
patent pivot
yeah i probably can
green oriole
I saw a reaction
sharp timber
I deleted it
patent pivot
where is the root logging instance
green oriole
bot.log?
subtle kraken
how come we never restrict any of our paginators yet the paginator doesn't error out?
Does None have id attribute afterall?
Ah I see
patent pivot
!int e ```py
print(bot.log)
In [1]: print(bot.log)
Out[1]:
File "/bot/bot/exts/utils/internal.py", line 178, in _eval
res = await func()
File "<string>", line 5, in func
AttributeError: 'Bot' object has no attribute 'log'```if not restrict_to_user: restrict_to_user = ctx.author
gritty wind
Yes?
green oriole
I meant the bot.log module
late wolf
If you make a command thorugh internal eval and the bot restarts does the command still presist or does it get lost?
subtle kraken
the pagination does feel like bunch of spaghetti pieced together
sharp timber
If you make a command thorugh internal eval and the bot restarts does the comman...
lost
late wolf
right
gritty wind
the pagination does feel like bunch of spaghetti pieced together
Some of it is years old ¯_(ツ)_/¯
green oriole
Wouldn't logging.getLogger().setLevel(5) work @patent pivot?
I think scragly is rewriting the paginator
patent pivot
!int e ```py
import logging
logging.getLogger().setLevel(5)
In [4]: import logging
...: logging.getLogger().setLevel(5)
...: ```and now we wait
but I'm p sure that wouldn't update the root logger
also fwiw the metricity PR works 👍
green oriole
Nice
sharp timber
Can someone with Help Cooldown try trashcanning a message
green oriole
It didn't seem to update the logging though
gritty wind
What are you guys hoping to see anyways?
gritty wind
Can someone with Help Cooldown try trashcanning a message
can you send one
sharp timber
!paste
stable mountainBOT
late wolf
!paste
gritty wind
cooldown
late wolf
ye so whats the issue?
sharp timber
sometimes it gets erased
green oriole
Only admins, mods and owners are moderation roles
Help Cooldown shouldn't affect that
Do we at least know which commands are affected?
late wolf
may I know whats wrong with the paste command?
gritty wind
Do we at least know which commands are affected?
Yes
all of them lol
there is a list of recorded instances in #dev-core
green oriole
may I know whats wrong with the paste command?
Trashcans seems to be working for some people when it shouldn't
Hmm okay
late wolf
oooh, but it didn't work for me
patent pivot
cooldown @late wolf
gritty wind
it isn't reliably reproducible
patent pivot
2021-01-30 13:34:04 2021-01-30 13:34:04 | bot.exts.info.tags | INFO | Inheritanc-e ♦#1090 tried to get the 'paste' tag, but the tag is on cooldown. Cooldown ends in 32.5 seconds.
late wolf
I was talking about the traschcan emoji
late wolf
even though i had cooldown it didn't get deleted
gritty wind
The cooldown role was just a theory
sharp timber
^
gritty wind
The reason is currently unknown
late wolf
sharp timber
It's not help channel restricted, probably
subtle kraken
did you figure out why everyone can delete the paginator yet?
green oriole
Mope
subtle kraken
well
green oriole
Yes, mope
subtle kraken
by the code we set it so anyone can delete them
sharp timber
uh
green oriole
Did we?
sharp timber
I think I found it
subtle kraken
!e
no_res = (
True
or True and False)
print(no_res)```@subtle kraken :white_check_mark: Your eval job has completed with return code 0.
True
with contextlib.suppress(asyncio.TimeoutError): await bot.instance.wait_for('reaction_add', check=check, timeout=timeout) await message.delete()
subtle kraken
no_restrictions = (
# The reaction was by a whitelisted user
user_.id == restrict_to_user.id
# The reaction was by a moderator
or isinstance(user_, Member) and any(role.id in MODERATION_ROLES for role in user_.roles)
)```That's expected though? If the user is the sender, it should delete
green oriole
It looks good to me
green oriole
Either the first line is true, or the second one
gritty wind
The conditions for deletion are: user_ is restrict_to_user or mod and member are true
gritty wind
> with contextlib.suppress(asyncio.TimeoutError):
> await bot.instan...
Is something wrong here?
subtle kraken
that code simply deletes when message is old so I'd doubt
gritty wind
Yes, but
subtle kraken
it does
sharp timber
And doesn't .suppress() simply then skip the delete check?
subtle kraken
why would it
gritty wind
once it does, it should delete the message
but the embeds are being deleted much earlier than expected
sharp timber
cause doesn't it jump to the end?
Return a context manager that suppresses any of the specified exceptions if they occur in the body of a with statement and then resumes execution with the first statement following the end of the with statement.
gritty wind
Once the timeout is done, that is the intended behavior
But the timeout is long enough that it shouldn't be the cause here
green oriole
The timeout is 5 minutes though
sharp timber
Right, but, then, are tags getting deleted if they're too old?
subtle kraken
okay am just pretty blind
sorry, they don't
the code only will delete message if check has passed and it did not time out
gritty wind
They aren't, the reported instance I've seen could happen up to a few minutes after they are sent
Well under the 5 minutes
green oriole
Do we even call this function? I don’t think we do with tags
sharp timber
Mine happened in seconds
subtle kraken
we paginate tags afaik
gritty wind
Yup, so the timeout isn't the cause
green oriole
It would be constant otherwise
gritty wind
And more importantly, I don't think we would've seen this spike in reports because that behavior is unchanged
sharp timber
Did someone just intentionally erase my !paste spam or did they all just clear themselves?
because that was within one minute
#bot-commands
cold island
We could whitelisted the whole category tbh (CC: <@512354988157173763>)
I don't mind, but I was wondering if it might somehow clash with us writing a channel name or something
sharp timber
Found a related issue
#bot-commands message
It has my reaction. I can't delete it.
Oh crap, I think I know what's wrong
late wolf
lol they all gone
green oriole
I don't mind, but I was wondering if it might somehow clash with us writing a ch...
That’s a good point
sharp timber
@green oriole see me in #bot-commands to watch this?
!paste
!paste
Ughl
A single reaction erases all of the embeds
late wolf
I wonder if its because you have moderation roles because it works fine for me
sharp timber
It's just erasing all of them
A single trashcan right now is erasing 10+ messages if a mod does it
late wolf
And its also deleting them even if no trashcan emoji is added
sharp timber
The reaction message check has to be failing for this to happen
What happens to reaction.message if the message is deleted?
!paste
shit
it happens cross-channel
@patent pivot
!tags
late wolf
also i can delete bast's tags command output
sharp timber
If only I had my test setup active, F**K
patent pivot
cross-channel huh
sharp timber
!paste
patent pivot
now that's weird
gritty wind
right that makes sense
patent pivot
lol
gritty wind
the event fires for all active ones for any emoji add 🤦♂️
sharp timber
I don't understand how
gritty wind
I guess a quick-fix would be to add a check that the message has the delete emoji from the author
patent pivot
lol
sharp timber
Are discord.Reaction instances being cached?
gritty wind
message x registers an event check
message y registers an event check
discord sends an event with trashcan emoji, and user of message x and y
sharp timber
We check the reaction.message.id against message.id
and message.id seems fine
if reaction.message is getting fucked up that would cause this
gritty wind
it isn't
the reaction.message is message x for x handler, and y for y handler
why, is a discord.py oddity probably
sharp timber
Well, the checks are passing, presumably
green oriole
A single trashcan right now is erasing 10+ messages if a mod does it
Ooooooh
gritty wind
https://github.com/Rapptz/discord.py/issues/6280
that seems to be an issue with members intents (in dms), is it applicable here
gritty wind
1.6
green oriole
I’ll push a fix to master, just need to add two parenthesis
gritty wind
Wait
sharp timber
?
cold island
That’s a good point
Tbh what I think is going to happen is that we get used to it and then try to use it in channels where it's not whitelisted and eventually add all staff channels. But we'll see
gritty wind
This still doesn't explain it happening to helpers
Unless mod reactions delete all embeds on the server
green oriole
Yep, a moderator reacting to any message will clear all the embeds on the server
green oriole
It will reset when redeploying
patent pivot
did that actually work
gritty wind
only one way to find out
patent pivot
lol no i mean the log level
gritty wind
There was nothing to log anyways
we don't have any logging that details the wait for
sharp timber
I’ll push a fix to master, just need to add two parenthesis
Did you figure it out?
green oriole
Nope
green oriole
Did you figure it out?
And yes
gritty wind
I have trace locally, and nothing shows up
sharp timber
What is it?
green oriole
https://github.com/python-discord/bot/commit/ced0af2b2936b1fab4f5601bd7b1c00bbbd9130a Basically the moderation role bypassed the message id check
sharp timber
Well, I was looking in all the wrong damn places
cold island
If it proves to be too much if an issue we can just disallow it for everyone. Mods can delete any message anyway
sharp timber
Yeah, that's the bug alright
gritty wind
If it proves to be too much if an issue we can just disallow it for everyone. Mo...
Its fine now
and python is back up, if anyone still wants to test it
stable mountainBOT
green oriole
!code
sharp timber
Yeah, confirmed fixed
green oriole
Awesome!
short snow
<@!736234578745884682> hey
Sorry, was not there.
patent pivot
all good!
short snow
(ping me when u decide what to whitelist.)
Any confirmation on this?
green oriole
Let's stick to the two channels
short snow
Ok.
gritty wind
We are having infra problems atm, thanks for the report
short snow
oh ok
vale ibex
.issue 1225 bot
vale ibex
What would be the preferred implementation for this?
I've found two possible solutions that seem correct
green oriole
Well, what are they? 
cold island
in the issue
vale ibex
Hah sorry, they're in the issue
One is in the error handler, the other in the converter
green oriole
Commented
vale ibex
Luckily that's the fix I still have stashed, so will just push it when I get home 😀
short snow
@vocal wolf will do that
vocal wolf
huh?
short snow
don't see a reason why we can't allow the player to set the bot's token as well.
@gritty wind what do u mean by this? Is to check whether the emojis are not equal or ....?
gritty wind
I mean you can just allow the function to take two tokens, like you do with the player vs player
tawdry vapor
You can ignore the errors about http_session and trace. PyCharm just cannot figure out that they exist since the attributes are added dynamically to instances of Bot and Logger respectively.
short snow
thought so, i add them to my personal project too, and pycharm couldn't figure them out
tawdry vapor
Well the trace is a more complicated monkeypatch but the principle is the same
short snow
@vocal wolf question, do i default the language to python in cht.sh command, or ask user for it
vocal wolf
default to py
short snow
ok
short snow
default to py
another question (sorry for disturbing.)
the default 404 Not Found looks like this: https://cheat.sh/python/abbbcccddddbbbee , and the response always returns status 200, so should i over-write this and let it display this
vocal wolf
resp.status is a 200?
short snow
# 404 NOT FOUND
#
# Unknown cheat sheet. Please try to reformulate your query.
# Query format:
/LANG/QUESTION
# Examples:
/python/read+json
/golang/run+external+program
/js/regex+search
# See /:help for more info.
#
# If the problem persists, file a GitHub issue at
# github.com/chubin/cheat.sh or ping @igor_chubin
yep
always 200
vocal wolf
uhhhhhhhh
short snow
i can check if it the response is starting with # 404 NOT FOUND and then send my own error embed
vocal wolf
should we wait until this is fixed?
vocal wolf
if it's a few months old then yeah we should implement our own way of detecting if a page cannot be found
vocal wolf
make sure to include the issue in the PR
short snow
ok
vocal wolf
how is this?
Looks pretty good I'd say
vocal wolf
do we do that with other request commands?
short snow
lemme see
vocal wolf
because I can't remember lol
vocal wolf
alright dew it
gritty bolt
<@!171159641416400897> check out your pr! Gave a review
Yo thanks. I had to meet a few school deadlines so I have been busy, but I will have time to look at them later today
And, what do you think of the bot?
short snow
And, what do you think of the bot?
i didn't get u
short snow
#sir-lancebot-playground message can we change the token and number colors?
short snow
my exams are coming near, so i wanna complete everything before 2nd Feb, Hence the prs are getting opened so fast.
also i won't be able to give much attention to them from 15th Feb to March 18th.
short snow
#sir-lancebot-playground message the pings are not being shown in a proper way
gritty wind
How are they supposed to look?
short snow
like my mention, it is not the code problem, but discords ig.
a better way is just their display names
gritty wind
You can't have mentions in embeds
I think its easier with mentions so we don't have to handle usernames, or nicknames or whatever
short snow
then how did my mention come? which didn't ping me
gritty wind
Mentions appear formatted but don't ping in embeds
short snow
but it is sometimes shown with ids, which is even more harder
.ttt log
see the first 1
gritty wind
The fix wasn't ported to sir-lance
and that's a problem on your end, probably need a cache refresh
short snow
ah ok
short snow
Chrisjil is ChrisLovering right?
vale ibex
That's me 🙂
short snow
ok. thanks for the quick review
and nice
that's a pretty good way using should do and could do
vale ibex
yea, I tend to use those, so that it sets the seen scene for the comment 😛
otherwise all my comments seem like they are things that must be done
vale ibex
Yea, and that's the big issue with should do/could do, everyone has different opinions on what is what
gritty bolt
For sir-lancebot, if the api does not return a 200 status, what error should I raise?
vale ibex
What's the context?
gritty bolt
Currently working on a valentines cog that returns a player a poem
From Shakespeare's poems
By using a Markov chain
The cog accesses an api that gives a list of rhymes for a given word
The cog will continue to sift through lines of Shakespeare's poems until it hits one has its last word inside the list of rhymes
But if the api is down, this shouldn't work
vale ibex
Is the only cause for an error if the API is down?
gritty bolt
Well, it can technically work because there is a memoize function in place
No, there are other ways for errors to occur
async with self.bot.http_session.get(
website + word,
timeout=10
) as response:
if response.status != 200: # 200 means 'ok'
logging.warning(
f"Received response {response.status} "
"from: {website + word}"
)
raise Exception
curr_set = set(
data["word"] for data in await response.json()
if data.get("score", 0) >= min_score
)
rhyme_set |= curr_set
So I am planning to raise an error, where I have put Exception
So this stops the entire process and tells the user that the api is down and poems are harder to generate
Should I create a custom Discord error for this?
And catch it inside the cog_command_error?
cold moon
Why this should raise error when API is down?
It's better to just send message about it to channel
gritty bolt
I am not sure of a better of stopping the whole process than to raise an error
This check happens inside the get_rhyme function
But it's one function inside a nest of other functions
This function is supposed to return a rhyme_set, I suppose I could return None and bubble up the error to the function responsible for returning the user a poem
And instead return an error message
But I figured that raising an error would be cleaner
gritty wind
Hmm this seems like an error localized to the poem-generator, so it would make sense for the error to handled within the file
If we handled all errors in the error handler, we’d have a monolithic file
vale ibex
.issue 1373 bot Do we want to tell users that the command is on cooldown, or not output anything at all?
dusky shoreBOT
[404] #1373 Issue/pull request not located! Please enter a valid number!
vale ibex
.issue 1373 bot
green oriole
I'd say not output anything to match the behavior of actually invoking the command
vale ibex
Alright, an easy fix for it was to change the return from display_tag to True if the command is on cooldown. It doesn't seem to have any side effects but it doesn't seem "right"
differentiating between not found, cooldown and success might require some sort of enum
gritty wind
Hmm if something matches a tag, we probably don’t want to even let it get through to the command sugegstor
vale ibex
wdym? py with contextlib.suppress(ResponseCodeError): if await ctx.invoke(tags_get_command, tag_name=tag_name): return -> ```py
with contextlib.suppress(ResponseCodeError):
await ctx.invoke(tags_get_command, tag_name=tag_name)
return
so it always returns early if the tag converter succeeds?
error_handler:168 for ref
This has the same effect as what I did, but we can keep the return for commands on cd as False
gritty wind
If the tag converter matches something, the intention was most likely a tag. In that case, we don’t really need to check for alt commands
I don’t remember the exact way it functions though, so I can’t say if that’s different from your solution
vale ibex
I think I prefer your proposed solution
Ahh, it looks like the tag converter just checks the tag name is a valid format, as opposed to an existing tag
gritty wind
Yeah the get function does the fuzzy matching
What I don’t remember if there is any way to check the fuzzy matching results without modifying the function
short snow
in a lot of places in the sir-lancebot code this happens:
def func() -> None:
return await ctx.send()
but it should be
def func() -> None:
await ctx.send()
return
It does the fuzzy matching in the block after, in error_handler. If the get function returns true though it quits early, so doesn't reach it
gritty wind
Can you link an example?
vale ibex
I'm feeling more like the get function should return true, even if the command is on cooldown
since it handles the cooldown itself anyway
and False is reserved for when there are no matches
gritty wind
Yeah that’s looking like the right move
Though it’s original intention was to indicate if something can be sent
And since it does the cool down checks internally “can be sent” also includes the cool down
short snow
@vale ibex here, got the old screenshot in my bin
With no headers
And i don’t see the need of line pagintor, just let is send plain text, and the bot can’t mention anyone , so the only problem is the content escapes the cod block, i could even add a fix for that using zero width spaces (that’s what it is called ig)
mossy wolf
that was one interesting experience, trying to rebase my project and having almost every single one of the 190 commits to master conflict lol
mossy wolf
i did a full rebase now but still this isn't working.....
not sure what is causing the commit fail
vale ibex
<@!126811506632294400> here, got the old screenshot in my bin
Ahh I see why now. because we're using a aiohttp.ClientSession() our user agent is actually Python/3.8 aiohttp/3.7.3
short snow
Yeah
short snow
Hmm, some core dev can just squash dev for u
brazen charm
You should just merge instead of rebasing new history
vale ibex
Yeah
https://github.com/chubin/cheat.sh/pull/280 see if this ever gets merged 😛
mossy wolf
there we go i think i managed to merge all the conflicts now
hmm flake8 is giving me trouble now. how does one use repr()?
as in like i'm trying to make a new tag, and i have a mix of single line and multi line code blocks
short snow
Can u show your repr code?
vale ibex
How did you create the tag? Did you make a new .md file in bot/resources/tags?
Those errors seem to suggest you created a .py file
mossy wolf
yes a new .md
i did start the project quite a while ago but got off tracked by school so didn't have time to finish it
so i decided to pick it up and rebase to get all the ~200 missed commits to master (resolve merge conflicts)
you reckon the repr() is inside some other code?
short snow
I would say to revert the merge into your branch commit, or redo the pull request from a new branch
mossy wolf
i was thinking about deleting the PR and branch and making a new branch from master yeah
short snow
the problem is not from your code, so i had say to leave that out
vale ibex
Yea, run git remote add upstream git@github.com:python-discord/bot.git then git fetch upstream then git pull upstream master
that should get you up to date
use https://github.com/python-discord/bot.git if you don't clone via ssh keys
green oriole
which you should totally be doing
vale ibex
short snow
I just reset my main always, and then rebase it into my branch
since main has 0 commits anyways
mossy wolf