Upgrade discord.py to v1.0.1#23523
Conversation
OverloadUT
left a comment
OverloadUT
left a comment
There was a problem hiding this comment.
Looks good to me. I haven't personally tested it; just reviewed the code.
fabaff
left a comment
fabaff
left a comment
There was a problem hiding this comment.
Looks good to me 🐦
Service call:
{
"message":"The sun is {% if is_state('sun.sun', 'above_horizon') %}up{% else %}down{% endif %}!",
"target":[
"3311xxxxxxxxx272"
],
"data":{
"images":[
"/home/ha_test_dev/.homeassistant/logo.png"
]
}
}
| images = list() | ||
|
|
||
| for image in data.get(ATTR_IMAGES): | ||
| if os.path.isfile(image): |
There was a problem hiding this comment.
This does I/O and we're in a coroutine. That combo is not allowed. Use hass.async_add_executor_job to schedule the I/O on the executor thread pool.
| if ATTR_DATA in kwargs: | ||
| data = kwargs.get(ATTR_DATA) | ||
|
|
||
| if ATTR_IMAGES in data: |
There was a problem hiding this comment.
This appears to have caused an unintended breaking change. If data is not in the service call it throws a NoneType error here. At minimum you must have data:{} now in order for the service call to be successful. Reported in issue #23948
There was a problem hiding this comment.
Yes I saw that only after I updated do 0.93, and was waiting for the weekend to fix it
7 participants
Description:
Upgrade discord.py to v1.0.1 to have Discord notify component working again
Related issue: fixes #21374
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
requirementsin the manifest (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices: