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

Telegram bot #27

Merged
merged 15 commits into from
Feb 17, 2019
Merged

Telegram bot #27

merged 15 commits into from
Feb 17, 2019

Conversation

toshka
Copy link
Contributor

@toshka toshka commented Feb 10, 2019

Hi.
Sometimes it is too lazy to open server console to add new torrent. So I decided to extend telegram bot capabilities. Now it is fully working and configurable but I have few open questions and hope you will help me.

  1. What is the best way to run bot on the server? Right now I temporary use screen, but we need some production-ready solution. Or at least describe solutions at README
  2. Documentation is not ready. I cant decide where to write it.

@idlesign
Copy link
Owner

Hi,

Some time ago I was thinking about daemon process, which ought to handle UI (CLI, Web, GTK) commands, maybe this is the way to go. Yet daemon and API needs to be designed. Such approach opens various possible capabilities, including rich UI and bot endpoints.

@NecroKote
Copy link
Contributor

NecroKote commented Feb 11, 2019

Hey folks,
why don't schedule torrt with some specific command like tgbot_pull, the same way current walk implemented?

(Uponn execution, torrt will pull new messages from TG and process them.)

Yes, that is not real-time experience bots usually have, but IMHO this way we get consistent experience, similar to current torrt workflow (cron-scheduling).
Also, it is pretty simple, and do not require additional layer of complexity (like residential daemon or API servvice).

@idlesign
Copy link
Owner

Hi,
Sure, that might work with getUpdates, but not webhooks (offering more responsive UX).

@NecroKote
Copy link
Contributor

IMHO webhooks, in this case, is a redundant feature.

The difference between adding torrent right away and with the delay of 1-2 min. is not that significant to complicate the existing solution.

torrt/bots/telegram_bot.py Outdated Show resolved Hide resolved
torrt/bots/telegram_bot.py Outdated Show resolved Hide resolved
@idlesign
Copy link
Owner

@toshka

  1. Let's settle with cron-based solution for now.
  2. I expect here https://github.com/idlesign/torrt/blob/master/docs/source/toolbox.rst and there https://github.com/idlesign/torrt/blob/master/docs/source/commands.rst

@NecroKote Deamon will be implemented someday if I had time %)

@NecroKote
Copy link
Contributor

@idlesign, if you'll have some time to describe your vision on 'daemonized' torrt, I'll be glad to look into a way of implementing it! Maybe we need new 'Issue' to start discussion ?

@toshka
Copy link
Contributor Author

toshka commented Feb 11, 2019

I thinking a lot about torrt daemon. Right now for me best solution is supervisor (http://supervisord.org/). Because:

  • It is has good documentation
  • It is easy configurable and don't uses much resources.
  • Finally, It is already installed at my server.

May be we just write some README file now?

@idlesign
Copy link
Owner

idlesign commented Feb 12, 2019

@NecroKote Some thoughts in #28

@toshka

Right now for me best solution is supervisor

Supervisord is a process control, but I'm talking about daemonization as in #28. Sure you would be able to use Supervisord or Upstart or Systemd, etc. %)

May be we just write some README file now?

Sure, we've already settled that the issue won't depend on daemon to be merged. You can update README with bot usage examples, but I'd rather expect them to be in the documentaion. A step by step description of how to configure and use bot commands would be very nice.

torrt/bots/telegram_bot.py Outdated Show resolved Hide resolved
torrt/bots/telegram_bot.py Outdated Show resolved Hide resolved
torrt/base_bot.py Show resolved Hide resolved
Repository owner deleted a comment from coveralls Feb 15, 2019
setup.py Outdated
@@ -48,6 +48,9 @@ def get_version():
],
setup_requires=[] + (['pytest-runner'] if 'test' in sys.argv else []) + [],

extras_require={
'telegram': ['python-telegram-bot']
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to pin down the lowest version number?

Copy link
Contributor Author

@toshka toshka Feb 15, 2019

Choose a reason for hiding this comment

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

Yes, but more important was to pin highest version because maintainers are working on major release version 12 with lot of changes.

Copy link
Owner

Choose a reason for hiding this comment

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

Are they already guilty in backward incompatible releases? %)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw a lot of warnings in console for beta version so decide exclude 12 version for now.

LOGGER.info('Telegram message was sent to user %s' % self.chat_id)
else:
LOGGER.error('Telegram notification not send: %s' % r['description'])
response = requests.post(url, data={'chat_id': self.chat_id, 'text': msg})
Copy link
Owner

@idlesign idlesign Feb 16, 2019

Choose a reason for hiding this comment

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

Please do not introduce unrelated changes into the pull request, instead make several requests.

Copy link
Contributor Author

@toshka toshka Feb 16, 2019

Choose a reason for hiding this comment

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

Sure, sorry

@idlesign idlesign merged commit 808942f into idlesign:master Feb 17, 2019
@idlesign
Copy link
Owner

Thank you. Merged.

@toshka toshka deleted the bot branch March 11, 2019 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants