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

Dynamic subscriptions and password-based authentication with Telegram #5238

Merged
merged 15 commits into from
Sep 6, 2016
Merged

Conversation

DBa2016
Copy link
Contributor

@DBa2016 DBa2016 commented Sep 6, 2016

Short Description:

  • implementing password-based authentication with TelegramTask (thereby circumventing the inability to get numeric ID from username)
  • implementing dynamic subscriptions (see docs/telegramtask.md for details)

Fixes/Resolves/Closes (please use correct syntax):

@DBa2016
Copy link
Contributor Author

DBa2016 commented Sep 6, 2016

Hmmm, what's up with Travis CI?...

@solderzzc
Copy link
Contributor

hmm don't know.

@solderzzc
Copy link
Contributor

solderzzc commented Sep 6, 2016

👍

Approved with PullApprove

@DBa2016
Copy link
Contributor Author

DBa2016 commented Sep 6, 2016

I had to rebase due to a commit made in the meantime...

@DBa2016
Copy link
Contributor Author

DBa2016 commented Sep 6, 2016

Tested with: no master at all, numeric master, wrong symbolic master and correct master. Worked so far for me.

Please note: master is case-sensitive.

@solderzzc
Copy link
Contributor

@DBa2016 It's time to merge. This is very good.

@solderzzc
Copy link
Contributor

solderzzc commented Sep 6, 2016

👍

Approved with PullApprove

@solderzzc solderzzc merged commit 1e62ec4 into PokemonGoF:dev Sep 6, 2016
@madzul
Copy link

madzul commented Sep 6, 2016

Got this error (after latest pull):

Traceback (most recent call last):
File "pokecli.py", line 803, in
main()
File "pokecli.py", line 150, in main
bot = start_bot(bot, config)
File "pokecli.py", line 108, in start_bot
initialize_task(bot, config)
File "pokecli.py", line 92, in initialize_task
tree = TreeConfigBuilder(bot, config.raw_tasks).build()
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/tree_config_builder.py", line 79, in build
instance = worker(self.bot, task_config)
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/base_task.py", line 23, in init
self.initialize()
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/cell_workers/telegram_task.py", line 21, in initialize
self.bot.event_manager.add_handler(TelegramHandler(self.bot, self.config))
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/event_handlers/telegram_handler.py", line 292, in init
initiator = TelegramDBInit(bot.database)
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/event_handlers/telegram_handler.py", line 264, in init
self.initDBstructure()
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/event_handlers/telegram_handler.py", line 276, in initDBstructure
self.initDBobject(objname, db_structure[objname])
File "/Users/madzul/work/PoGoBot/PGB-Dev/pokemongo_bot/event_handlers/telegram_handler.py", line 282, in initDBobject
if len(res) > 0 and res[0] != sql: # object exists and sql not matching
TypeError: object of type 'NoneType' has no len()

@DBa2016
Copy link
Contributor Author

DBa2016 commented Sep 6, 2016

Fixed by #5252, thanks to @rbusquet

@rbusquet
Copy link
Contributor

rbusquet commented Sep 6, 2016

I was searching for an issue/pull dealing with this problem. I'm glad that helped ^^

@Gobberwart
Copy link
Contributor

Gobberwart commented Sep 6, 2016

Looking good, but I've encountered a problem:

"master": gobberwart
no password set

Deleted all telegram tables from database and restarted bot.

Tried sending message from Telegram. Response: please /login first

[2016-09-07 07:17:15] [MainThread] [PokemonGoBot] [INFO] Telegram message from gobberwart (244691262): /info
[2016-09-07 07:17:15] [MainThread] [PokemonGoBot] [ERROR] Telegram message received from unknown sender. Please either make sure your username or ID is in TelegramTask/master, or a password is set in TelegramTask section and /login is is

I removed the "@" (this is no longer supported??) and the bot logged:

[2016-09-07 07:21:33] [MainThread] [PokemonGoBot] [WARNING] Telegram message received from correct user, but master is not numeric, updating datastore.

but no response received.

Tried "/info" again and received response: "No password nor master configured..." message.

Restarted bot and now I'm getting responses properly.

Something's not quite right.

@Gobberwart
Copy link
Contributor

Authentication by password works perfectly.

@madzul
Copy link

madzul commented Sep 7, 2016

Still got "No password nor master configured in TelegramTask section, bot will not accept any commands"
Mine is latest commit (1b8cf13)

how's the right example TelegramTask config?

@solderzzc
Copy link
Contributor

Seems current telegram bot is hard to use.

@rawgni
Copy link
Contributor

rawgni commented Sep 7, 2016

Maybe should stick with 1 way of configuring the telegram bot, ie. either using master or password.
The simplest is probably just specify a password, then msg the bot with the password and you are all set.

@Gobberwart
Copy link
Contributor

I tend to agree @rawgni. Password works perfectly. Why keep master userid/password as well?

@DBa2016 DBa2016 deleted the mydev branch September 7, 2016 11:24
@DBa2016
Copy link
Contributor Author

DBa2016 commented Sep 8, 2016

@Gobberwart , @rawgni : I am hesitant to remove a working method ("master" authentication), this would break working configs - therefore worked around to make both work (not sure why it did not work for your "master", @Gobberwart , can you check the case of your username? is it really all-small?).

But yes, I think password is easier and more flexible to use.

@Gobberwart
Copy link
Contributor

@DBa2016 I hear you, but I think at some point we have to bite the bullet and say, "This method is shit and confuses people. Let's ditch it and go with what works."

@Gobberwart
Copy link
Contributor

We can always add a log message "master in TelegramTask is deprecated, must use password instead"

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.

6 participants