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.

Sign up for GitHub

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

Jump to bottom

Change telegram library to pytelegrambotapi #6889

Merged
merged 9 commits into from
Dec 11, 2020
Merged

Change telegram library to pytelegrambotapi #6889

merged 9 commits into from
Dec 11, 2020

Conversation

rgstephens
Copy link
Contributor

Proposed changes:
Change Telegram library to address #6814.

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@rgstephens rgstephens requested review from alwx and erohmensing and removed request for alwx October 3, 2020 00:00
erohmensing
erohmensing approved these changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@erohmensing erohmensing left a comment

Choose a reason for hiding this comment

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

A few comments to address before merging, but seems to work as expected

rasa/core/channels/telegram.py Show resolved Hide resolved
rasa/core/channels/telegram.py Outdated Show resolved Hide resolved
rasa/core/channels/telegram.py Show resolved Hide resolved
rasa/core/channels/telegram.py Show resolved Hide resolved
rasa/core/channels/telegram.py Outdated Show resolved Hide resolved
rasa/core/channels/telegram.py Outdated Show resolved Hide resolved
rasa/core/channels/telegram.py Outdated Show resolved Hide resolved
erohmensing
erohmensing reviewed Oct 5, 2020
View reviewed changes
changelog/6814.improvement.md Outdated Show resolved Hide resolved
docs/user-guide/connectors/telegram.rst Outdated Show resolved Hide resolved
@rgstephens
Copy link
Contributor Author

@erohmensing I made changes and did full testing of the buttons based on your comments (thanks) but the test for the setWebhook endpoint is now failing. The old library has an alias for the setWebhook function but it seems to me that the test shouldn't care that the class alias is missing as long as the setWebhook endpoint is supported and there's been no change to the blueprint for setWebhook. I'm thinking the test needs to be changed but would like your thoughts on this. -Thanks.

erohmensing
erohmensing approved these changes Dec 4, 2020
View reviewed changes
docs/user-guide/connectors/telegram.rst Outdated Show resolved Hide resolved
docs/user-guide/connectors/telegram.rst Outdated Show resolved Hide resolved
docs/user-guide/connectors/telegram.rst Outdated Show resolved Hide resolved
docs/user-guide/connectors/telegram.rst Outdated Show resolved Hide resolved
docs/user-guide/connectors/telegram.rst Outdated Show resolved Hide resolved
@rgstephens rgstephens merged commit b18f700 into 1.10.x Dec 11, 2020
@rgstephens rgstephens deleted the telegram-pytelegrambotapi-1.10 branch December 11, 2020 18:12
@rgstephens rgstephens added this to the 1.10.19 Rasa Open Source milestone Dec 15, 2020
@wochinge wochinge mentioned this pull request Dec 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erohmensing erohmensing erohmensing approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.10.19 Rasa Open Source
Development

Successfully merging this pull request may close these issues.
2 participants
@rgstephens @erohmensing