add notes on running functions instead of setting post_ methods#3797
add notes on running functions instead of setting post_ methods#3797Bibo-Joshi merged 5 commits intodoc-fixesfrom
post_ methods#3797Conversation
telegram/ext/_applicationbuilder.py
Outdated
| :any:`Custom Webhook Bot Example <examples.customwebhookbot>`), | ||
| make sure that you explicitly call this method. |
There was a problem hiding this comment.
| :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?
There was a problem hiding this comment.
Hmm... makes sense :)
As for the insertion - it doesn't work if I mention methods there. Sphinx complains about it.
There was a problem hiding this comment.
ah, you'll have to use the full name then, see e.g. :meth:~telegram.ext.Application.run_polling
There was a problem hiding this comment.
OK, done. It didn't like the :any: link to the example either, so I had to put an https hyperlink instead
post_ methods explicitlypost_ methods
harshil21
left a comment
There was a problem hiding this comment.
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.
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 |
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)? |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
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 …
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.