[add] Add Telegram Input Plugin#2861
Conversation
|
I see now that It's returning errors in github validating the schema, but locally I don't have any kind of errors! |
| append = True | ||
| break | ||
|
|
||
| if not append: |
There was a problem hiding this comment.
instead of using a control variable, you can use the else clause for the for loop that will be called if it reached its end (without breaking or returning earlier)
|
|
||
| #Check Types | ||
| if types: | ||
| if message['message']['chat']['type'] not in types: |
| break | ||
|
|
||
| #Append the entry | ||
| if append: |
There was a problem hiding this comment.
please use the for/else pattern instead of a control variable
There was a problem hiding this comment.
I can't use the else here because I need to run the complete loop break it if any check fails! The else in this case will make the code more complicated, because I would need to append the entry in the else of the for, and then in the case of no entry_config is informed
|
It's unclear to me what kind of data this plugin will be used to pass into the task |
|
Naming is not following convention. We don't use "input" as a part of plugin name. |
| entry: | ||
| <field>: <regexp to match value> | ||
|
|
||
| Note: If not declated, title will be the message |
|
14+ reviews! Noob alert! 😊 This is my first time playing with python, I'm still catching up on some conventions! I'll review all comments tomorrow! Thank you! |
| append = False | ||
| for i in whitelist: | ||
| if 'username' in i and i['username'] == message['message']['from']['username']: | ||
| logger.debug("WhiteListing: Username {}",message['message']['from']['username']) |
There was a problem hiding this comment.
If you repeatedly use long access like message['message']['from']['username'], please extract that into local variable instead.
|
I reviewed the code and I'll add relevante notes in the change requests as comments |
I renamed the task to "from_telegram" I can't use only telegram because a plugin with that name already exists in the notify! |
Basically anything that you desire... You can ask to download a movie based on name, add a new series to a entry list, forget a episode, run a task.... It's all a mater of matching the correct statement and execute the task based on that. One thing that would be great is to this plugin to inform the telegram from notify of the target to where send a notification. That way you could ask for something in this plugin and then receive the notification in the same chat! That way anyone autorized could chat with flexget and request something. That integration can be added later |
|
@liiight and @paranoidi code and rename ready for review, when you have the time for it! Thank you |
liiight
left a comment
There was a problem hiding this comment.
thanks, a few more changes requested
| only_new = config['only_new'] | ||
| entry_config = config.get('entry') | ||
| whitelist = config.get('whitelist',[]) | ||
| types = config.get('types') |
There was a problem hiding this comment.
you can set the default here:
| types = config.get('types') | |
| types = config.get('types', ['private', 'group']) |
| types = ['private', 'group'] | ||
|
|
||
| #Get Last Checked ID | ||
| update_id = task.simple_persistence.get(token+'_update_id') |
| ).json() | ||
| except HTTPError as e: | ||
| raise plugin.PluginError("Error getting telegram update: {}" % e) | ||
| return |
There was a problem hiding this comment.
no need to return after raise, it's unreachable
|
|
||
| #We have a error | ||
| if not response['ok']: | ||
| raise plugin.PluginError("Telegram updater returned error {}: {}" % (response['error_code'], response['description'] ) ) |
There was a problem hiding this comment.
this isn't a logger, please use fstring
| task.simple_persistence[token+'_update_id'] = update_id | ||
|
|
||
| #We Don't care if it's not a message or no text | ||
| if 'message' not in message or 'text' not in message['message'] or 'chat' not in message['message'] or 'type' not in message['message']['chat']: |
There was a problem hiding this comment.
this line is way too long, please run black
| entry['title'] = text | ||
|
|
||
| #We need a url, so we add a dummy | ||
| entry['url'] = "http://localhost?update_id="+str(update_id) |
| entry['telegram_message'] = message['message'] | ||
|
|
||
| #Check Types | ||
| if types and message['message']['chat']['type'] not in types: |
There was a problem hiding this comment.
this check should probably happen sooner
| #Process the entry config | ||
| accept = True | ||
| if not entry_config: | ||
| pass |
There was a problem hiding this comment.
Removed the pass! Done!
|
All reviews done! Hope that all is ok now! Tks |
|
Run black in the plugin |
liiight
left a comment
There was a problem hiding this comment.
Great job dude, almost there ;-)
Please go over all string appending, use fstrings everywhere except log messages
| types = config.get('types', ['private', 'group']) | ||
|
|
||
| # Get Last Checked ID | ||
| update_id = task.simple_persistence.get('{}_update_id'.format(token)) |
There was a problem hiding this comment.
please use fstrings
| update_id = task.simple_persistence.get('{}_update_id'.format(token)) | |
| update_id = task.simple_persistence.get(f'{token}_update_id') |
There was a problem hiding this comment.
Done, I released now that I was using a older fstring format, this one is much better. Tks
| params['offset'] = update_id | ||
|
|
||
| # The Target URL | ||
| url = "{domain}bot{token}/getUpdates".format(domain=_TELEGRAM_API_URL, token=token) |
There was a problem hiding this comment.
please use fstring
| url = "{domain}bot{token}/getUpdates".format(domain=_TELEGRAM_API_URL, token=token) | |
| url = "f{_TELEGRAM_API_URL}bot{token}/getUpdates" |
| try: | ||
| response = task.requests.get(url, timeout=60, raise_status=False, params=params).json() | ||
| except HTTPError as e: | ||
| raise plugin.PluginError("Error getting telegram update: {}".format(e)) |
|
Done!!..... 😅 |
liiight
left a comment
There was a problem hiding this comment.
LGTM, nice work on this dude!
|
|
||
| # Get Telegram Updates | ||
| try: | ||
| response = task.requests.get(url, timeout=60, raise_status=False, params=params).json() |
There was a problem hiding this comment.
just noticed this, but if you set raise_status=False it won't raise an exception here. you need to set it to True (which is the default I think)
There was a problem hiding this comment.
Done @liiight ! Tks for the time spend on this
|
Thanks for this! Could you please update the wiki? |
Motivation for changes:
Add the ability to input entries from a telegram chat
Detailed changes:
Config usage if relevant (new plugin or updated schema):
secrets.yml
config.yml
Log and/or tests output (preferably both):
To Do: