Skip to content
This repository has been archived by the owner on Sep 30, 2022. It is now read-only.

Use edit message in help and settings #47

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AyraHikari
Copy link

Signed-off-by: Ayra Hikari [email protected]

@AyraHikari
Copy link
Author

AyraHikari commented Sep 24, 2018

I've added error flood control handling in help and settings button, if got error Flood control exceeded, it will edit message to Flood control exceeded. Retry in xxx seconds
And i think it handling other exception error 🤔

@PaulSonOfLars
Copy link
Owner

PaulSonOfLars commented Sep 24, 2018

... So your way of handling flood control is to edit to flood control exceeded? How does that make sense? You've hit flooding, you can't change it anymore? Or am I misunderstanding.

Copy link
Owner

@PaulSonOfLars PaulSonOfLars left a comment

Choose a reason for hiding this comment

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

Why are you catching exception? This is bad practice. And please make sure you adhere to PEP8.


# ensure no spinny white circle
bot.answer_callback_query(query.id)
query.message.delete()
except BadRequest as excp:
except Exception as excp:
if excp.message == "Message is not modified":
Copy link
Owner

Choose a reason for hiding this comment

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

Never, ever, EVER catch exception. You can have no idea what you're catching.


elif next_match:
chat_id = next_match.group(1)
next_page = int(next_match.group(2))
chat = bot.get_chat(chat_id)
query.message.reply_text("Hi there! There are quite a few settings for {} - go ahead and pick what "
bot.edit_message_text(chat_id=query.message.chat_id,
message_id=query.message.message_id,
Copy link
Owner

Choose a reason for hiding this comment

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

What are you doing with your indent? This isn't indented to pep8

@AyraHikari
Copy link
Author

Oh yeah sorry about that, fixed.
And about handling flood limit, when someone just clicking button again and again until reach flood limit, it will raise exception, and will not update or send message. Because telegram spam protection maybe.
But it work on another user who haven't flood limit in bot

Copy link
Owner

@PaulSonOfLars PaulSonOfLars left a comment

Choose a reason for hiding this comment

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

What is the overall aim of this PR? And how does it react when you press an old button? Bots cannot edit older messages (I think it's 2 days), this code currently just dies quietly and doesn't update the message.
You're just changing from send_message to edit_message (which I think has a lower flood limit). What are the advantages? You save a single delete api call, but that delete ensures that the help still works with older messages.

@@ -342,6 +359,9 @@ def settings_button(bot: Bot, update: Update):
elif excp.message == "Message can't be deleted":
pass
else:
bot.edit_message_text(chat_id=query.message.chat_id,
Copy link
Owner

Choose a reason for hiding this comment

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

So in conclusion, this line is useless and never succeeds then?

reply_markup=InlineKeyboardMarkup(
[[InlineKeyboardButton(text="Back",
callback_data="stngs_back({})".format(chat_id))]]))
bot.edit_message_text(chat_id=query.message.chat_id,
Copy link
Owner

Choose a reason for hiding this comment

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

Why not use query.message.edit_text?

@AyraHikari
Copy link
Author

It still work and updated when i press button on old message (2 month ago, may 17) based on my bot.
And i dont know edit message is have lower flood limit, but i think maybe it better performance?

@PaulSonOfLars
Copy link
Owner

Why would it be better performance? I'd like to have some sort of proof that edit has better or equal flood limit, and what your performance claim is based on, other than removing one delete api call.

@AyraHikari
Copy link
Author

I think it equal enough, because i clicked button in my bot and marie 19 times, and it not responding because flood limit. so it stuck at 19 times.
I can't compare about performance, because i have slow server.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Ok

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

Successfully merging this pull request may close these issues.

None yet

2 participants