Member
As discussed in dev group, I added a note stating that in cases like custom webhook example user should explicitly call their own functions instead of setting post_ methods.
Comment on lines +1099 to +1100
| :any:`Custom Webhook Bot Example <examples.customwebhookbot>`), | ||
| make sure that you explicitly call this method. |
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
| :any:`Custom Webhook Bot Example <examples.customwebhookbot>`), | |
| make sure that you explicitly call this method. | |
| :any:`Custom Webhook Bot Example <examples.customwebhookbot>`),. |
There is not really a need to set the function on the ApplicationBuilder in this case. you can just manually do await some_function(application) if you're doing custom logic anyways :)
Can we use a sphinx insertion thingy to avoid repitition?
Member Author
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... makes sense :)
As for the insertion - it doesn't work if I mention methods there. Sphinx complains about it.
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, you'll have to use the full name then, see e.g. :meth:~telegram.ext.Application.run_polling
Member Author
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, done. It didn't like the :any: link to the example either, so I had to put an https hyperlink instead
add notes on running
add notes on running functions instead of setting post_ methods explicitlypost_ methods
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh I think the note is redundant. The opening paragraph in post_* methods already tell that it is only executed by run_polling/webhook, and clicking on that link shows the order of execution ("The order of run_polling is as follows" line).
How do you feel about raising a warning if app.start() was called and app.post_init() was set but not called? This would need additional logic to work (some class variables we should set when the function has finished running?), so it might be worth discussing if we even want that.
Member Author
tbh I think the note is redundant. The opening paragraph in
post_*methods already tell that it is only executed byrun_polling/webhook, and clicking on that link shows the order of execution ("The order of run_polling is as follows" line).
OK, I see what you mean. I actually don't insist on adding my note. I realize now that by following this path, I could have figured out earlier that I have to await my function manually. I guess it's once again about our expectations from the user and how much "superfluous" (with or without quotation marks) information we want to give them in our docs. I definitely see that what is said in the note can be inferred from the info already present.
Member Author
How do you feel about raising a warning if
app.start()was called andapp.post_init()was set but not called? This would need additional logic to work (some class variables we should set when the function has finished running?), so it might be worth discussing if we even want that.
That's an interesting idea, similar to a warning when user has declared some states but is not using some of them. I too think, however, that the implementation will be rather tedious. Do we have any idea about how many users actually implement this custom logic (e.g. because of several webhooks like in my case)?
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with the proposed content changes as they are and I personally don't find them way too verbose (although @harshil21 ofc made a valid point).
I would guess that programmatically checking and issuing a warning is not worth the effort …
Merged
3 tasks