Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replaced telegram with an async version #11552

Merged
merged 7 commits into from Sep 9, 2022
Merged

replaced telegram with an async version #11552

merged 7 commits into from Sep 9, 2022

Conversation

tmbo
Copy link
Member

@tmbo tmbo commented Sep 8, 2022

Proposed changes:

  • use async communication when talking to telegram to avoid blocking sanic workers

@tmbo tmbo requested a review from a team as a code owner September 8, 2022 11:17
@tmbo tmbo requested review from znat and rasa-jmac and removed request for a team September 8, 2022 11:17
Copy link
Member

@ancalita ancalita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚡ Nice one!

rasa/core/channels/telegram.py Show resolved Hide resolved
rasa/core/channels/telegram.py Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

🚀 A preview of the docs have been deployed at the following URL: https://11552--rasahq-docs-rasa-v2.netlify.app/docs/rasa

Copy link
Member Author

@tmbo tmbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great, thanks a lot 👍 (I can't approve it since I created the PR but please go ahead and merge it)

@ancalita ancalita merged commit ed7d39e into 3.2.x Sep 9, 2022
@ancalita ancalita deleted the async-telegram branch September 9, 2022 14:53
@benos
Copy link

benos commented Oct 14, 2022

Hi,

This is failing for me. I get the following error when rasa starts:

image

And the bot is unresponsive when I type something in Telegram. Downgrading to 3.2.9 fixes the problem.

My credentials look like this:

image

Thanks!

Ben

@ancalita
Copy link
Member

@benos thanks for flagging! I'll open a fix PR asap for this on 3.3.x and hopefully that should fix this issue.

@benos
Copy link

benos commented Nov 18, 2022

Hi @ancalita

That sounds great, but do you have any idea when this fix might be made live? I don't see any change on 3.3.1

Thanks!

Ben

@ancalita
Copy link
Member

@benos Apologies for the delay, other priorities took hold 🙈
I've opened this PR, once it gets reviewed and merged, I'll release a new patch 🤞🏻

@benos
Copy link

benos commented Dec 4, 2022

@ancalita Awesome! Many thanks

@benos
Copy link

benos commented Jan 29, 2023

Hi @ancalita,

Sorry to reopen this, but this change has created an additional bug. When triggering an intent like so:

http://localhost:5005/conversations/[redacted]/trigger_intent?output_channel=telegram

This fails with the following error:

{"version":"3.4.1","status":"failure","message":"An unexpected error occurred. Error: asyncio.run() cannot be called from a running event loop","reason":"ConversationError","details":{},"help":null,"code":500}

I assume get_output_channel is re-run by the triggering code.

Best,

Ben

@benos
Copy link

benos commented Jan 29, 2023

@ancalita While I'm at it, might this line cause a leak if called repeatedly?

channel = TelegramOutput(self.access_token)

I've actually noticed a steadily rising CPU consumption which seems linked to calling trigger_intent, but could never really pinpoint the problem.

@ancalita
Copy link
Member

ancalita commented Feb 2, 2023

@benos I'm so sorry to hear about these new difficulties 🙈 I didn't envisage the potential issues of this change when using the HTTP API. Could you please create a ticket in this Jira board, the new home for OSS issues, with steps for reproducing the error.

I've actually noticed a steadily rising CPU consumption which seems linked to calling trigger_intent

A separate ticket for this would be welcome too 🙏🏻 with the reference to the particular line in the codebase if possible. Then you can link these two tickets as related, so they can inform each other.

@benos
Copy link

benos commented Feb 4, 2023

Hi @ancalita ,

I created two tickets:

https://rasa-open-source.atlassian.net/jira/software/c/projects/OSS/boards/1?modal=detail&selectedIssue=OSS-677&quickFilter=5

https://rasa-open-source.atlassian.net/jira/software/c/projects/OSS/boards/1?modal=detail&selectedIssue=OSS-676&quickFilter=5

The silver lining could be that this might have revealed the origin of this CPU link that has been plaguing us since we launched.

Best,

Ben

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants