Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • May 21 11:26
    codecov[bot] commented #1927
  • May 21 11:25
    codecov[bot] commented #1927
  • May 21 11:25
    codecov[bot] commented #1927
  • May 21 11:24
    codecov[bot] commented #1927
  • May 21 11:24
    codecov[bot] commented #1927
  • May 21 11:24
    codecov[bot] commented #1927
  • May 21 11:19
    github-actions[bot] labeled #1927
  • May 21 11:19
    github-actions[bot] labeled #1927
  • May 21 11:18
    st01cs opened #1927
  • May 20 10:15
    jacobtomlinson labeled #1926
  • May 20 10:15
    jacobtomlinson labeled #1926
  • May 20 10:15
    jacobtomlinson opened #1926
  • May 20 10:15
    jacobtomlinson labeled #1926
  • May 18 10:13
    codecov[bot] commented #1875
  • May 18 10:13
    codecov[bot] commented #1875
  • May 18 10:12
    codecov[bot] commented #1875
  • May 18 10:11
    codecov[bot] commented #1875
  • May 18 10:10
    codecov[bot] commented #1875
  • May 18 10:04
    codecov[bot] commented #1875
  • May 18 10:04
    FabioRosado synchronize #1875
cognifloyd (Jacob Floyd)
@cognifloyd:matrix.org
[m]
Woohoo! I just merged the teams PR! Thank you Jacob Tomlinson and FabioRosado for figuring out how to add teams support!
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Thanks for all the help reviewing. And your work on the test machinery. Tests have been the main blocker for the PR (and time) but that was a joy to test.
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Maybe I'm missing something, but if I change this line https://github.com/opsdroid/opsdroid/blob/master/opsdroid/connector/github/tests/test_connector_github.py#L237 to assert event.abc == "pyup-bot" and then run pytest opsdroid/connector/github/tests/test_connector_github.py::test_pr_merged it still passes. Any reason why this would be?
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Oooh interesting. I think because the assertions are inside the skill they get ignored.
We log exceptions in skills and move on. Don't want user code to cause the whole thing to die. https://github.com/opsdroid/opsdroid/blob/a45490d1bdceca39b49880e20262b55ea0be101d/opsdroid/core.py#L466
So we should probably be asserting that Exception when running skill is not logged once the skill has been run.
Good catch thanks!
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Yes, that totally makes sense. Hmmmm, I understand this, but in terms of implementation, that's like saying we should put the call_endpoint in a try and then catch the exception coming back, yah? If not, lemme know if you can pseudocode it out for me, and I'll try making the change
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Oho. Okay, I'll see if I can get that working for you. Thanks for much for the pointer.... learning :)
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
👍
We probably need to do this in every test that has assertions inside the skill
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Yup. If I get it working for one, I'll try to do it everywhere :)
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Awesome
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Jacob Tomlinson: so I am not convinced the skill-based tests are working. Here is what I mean by this... I couldn't get the caplog stuff working. But I read all about caplog and there's no reason it shouldn't work. So I tried just straight raising an exception in the skill, and finally just tried putting a breakpoint (like a pdb.set_trace()) in the skill... and none of it registered. There didn't seem to be any change I could make in the skill that would affect anything at all. I tried this with multiple skills. I only tried this with skills in the github connector so far.
Can you (or anyone for that matter) give this a try and see if you are getting something different from me? Maybe I'm doing something wrong.
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
I think you're right. I'm not sure the skills themselves ever get executed. Opsdroid adds the skill to the event loop and returns a 201 to let you know the execution has been created. But it doesn't wait for the execution to finish, this is because users could put all sorts in their skill and if it takes too long the GitHub webhook will time out.
Really we need something after the assert resp.status == 201 which waits for the skill to be called. Maybe even adding an asyncio.sleep after will be enough to get it to hit your breakpoint. But we probably need some testing utility which blocks until the skill has been called.
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Looks like there was a bug in the GitHub connector which did mean the skill was never called. I've opened opsdroid/opsdroid#1808 to fix it. I also updated the test to check that the skill was called.
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
With that bug fixed things are as we thought. The assertions in the skill fail but opsdroid continues anyway and the test passes. Checking the log and asserting that no exception was raised fixes that, so if you break the assertion the test fails. https://github.com/opsdroid/opsdroid/pull/1808/commits/d8dd73b9a70ba6198cc460740f17373f8f39a3aa
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Yesssss, awesome. Good stuff! Also, glad it wasn’t something deeper as I had originally suspected. Will officially review shortly :)
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Jacob Tomlinson: your PR appears ready to merge 😃
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Jacob Tomlinson: thanks to the path you paved, I added caplog capability to all the other Github skills based tests: opsdroid/opsdroid#1811
Ajit D'Sa
@ajitdsa:matrix.org
[m]
and while we're at it... another small one: opsdroid/opsdroid#1812
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Awesome thanks so much! Merging the first has made conflicts in the second. Once you've addressed those I'll have a look.
1 reply
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Jacob Tomlinson: I think since you merged opsdroid/opsdroid#1820 we can just close opsdroid/opsdroid#1794
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
👍
Ajit D'Sa
@ajitdsa:matrix.org
[m]

I have a question about the Github connector. I'm trying to make some modifications to it just to test some stuff out. I make my modifications and run opsdroid with a Github skill set up for the PRClosed event.

I close a PR in my repo which runs the skill. I can tell the skill is initially triggered because I see this message in my opsdroid log: 2021-08-22 23:41:33,955 INFO aiohttp.access: 127.0.0.1 [23/Aug/2021:04:41:33 +0000] "POST /connector/github HTTP/1.1" 201 191 "-" "GitHub-Hookshot/59a1e9d". That implies to me that this line was executed: https://github.com/opsdroid/opsdroid/blob/master/opsdroid/connector/github/__init__.py#L346

However, the code in my skill itself is never run. My guess is that due to the changes I made, there is an error somewhere up the chain in opsdroid itself before my actual skill is run. Something is failing, but there appears to be some sort of rescue in place so that the entire opsdroid itself doesn't die. If that's the case, there is no message being logged, so I can't tell where control goes after the line I pasted above.

Can anyone tell me if they think I'm even on the right path?

3 replies
Documentation Bot
@documentation-bot:cadair.com
[m]
Whoops there has been an error.
Check the log for details.
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Jacob Tomlinson: Are you kidding me? Yes, that was the problem. There was an error in what I was importing in the skill. FIxed that and now back to good. Thank you so much.
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Do you see any opportunities to add some more logging anywhere to help others avoid this in the future?
1 reply
Ajit D'Sa
@ajitdsa:matrix.org
[m]

Hehe I’m looking into that exact thing :) Going to finish my current train of thought, and then recreate the issue and either get some logging going or suggest what might have helped.

Thanks again.

Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
👍
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Maybe there's an opportunity to add something in the docs about this. For me, I think my expectation was that app would have just errored out and stopped working rather than just throw an error but continue on. I didn't realize that might be the case until i was literally typing my original message in this channel. I'll take a look and potentially make a suggestion if I think it would help.
Sleuth
@sloth56:chat.mountainview.theworkpc.com
[m]
Opsdroid doesn't crash out when a skill causes a crash when other skills are running. And sometimes not even then.
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Yes, I get that now. :) And it totally makes sense. I'm just trying to think of what may help other developers who don't realize this when they are working with opsdroid in the future.
Sleuth
@sloth56:chat.mountainview.theworkpc.com
[m]
Yeah, I hear you. Opsdroid is very hard to debug for sure.
Ajit D'Sa
@ajitdsa:matrix.org
[m]
Well now that I get it, I'm having a great time 😀 But I wasn't familiar with opsdroid (or python really) before, so I was just easily confused
Sleuth
@sloth56:chat.mountainview.theworkpc.com
[m]
Yeah, I think everyone has that ah-ha moment. I definitely did.
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
I'd love to hear thoughts about how we can improve this, I totally get that it can be a pain. If your skill errors it shows in the log. I'm pretty liberal with breakpoints when I'm debugging. But what would folks like to see?
Ajit D'Sa
@ajitdsa:matrix.org
[m]
No it’s exactly what I would like to see. I just missed it. That’s totally my fault. That’s why I’m advocating for something in the docs for writing/understanding skills that implies that if your skill fails, you will get an error in the logs, but the app won’t necessarily die.
Jacob Tomlinson (Slack)
@_slack_opsdroid_U5MK1BW83:matrix.org
[m]
Sure fair enough. More docs are always good!
Sleuth
@sloth56:chat.mountainview.theworkpc.com
[m]
I think what's needed is a start to finish writing your first skill guide. And a cheat sheet for common use cases.
Ajit D'Sa
@ajitdsa:matrix.org
[m]
I have a request for some testing help for decorators and asyncpg mocking if anyone has any experience or insight that might be helpful: https://github.com/opsdroid/opsdroid/pull/1805#issuecomment-927864969 ... then I feel we would be able to merge this PR (pending feedback from others, which I'm more than happy to receive)
Sleuth
@sloth56:chat.mountainview.theworkpc.com
[m]
Cadair: Is the matrix connector documented somewhere? Functions to create rooms, invite users, etc? If not it would be a good idea to have them.
Cadair
@cadair:cadair.com
[m]
Almost all the events supported are implemented, there is an issue tracking docs in general
Sleuth
@sloth56:chat.mountainview.theworkpc.com
[m]
Cadair: When you have a second can you comment on this? https://github.com/opsdroid/opsdroid/issues/1792#issuecomment-931506914
FabioRosado
@fabiorosado:matrix.org
[m]

Im picking up the slack connector after a long time and seems like the bot is ignoring all the breakpoints 🤔

Anyone has a vscode debug settings file ?