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

Provide Versions of customwebhookbot.py with Different Frameworks #3820

Merged
merged 16 commits into from Aug 16, 2023

Conversation

Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Jul 29, 2023

Closes #3717 by providing versions with flask, quart and Django. Other examples can ofc be added later as well.

I have used neither of the frameworks before, so the examples are surely not optimal from their perspective and I'm happy for improving comments. Especially for Django I am sure that an actual Django project would set this up very differently as (AFAIK) Django usually makes heavy use of several modules & files.

Moreover, I want to point out that ASGI servers like uvicorn are more frequently started from the command line than from within a python script, but I kept to this scheme because

  1. that way we don't have to provide additional info on how to run the script (it's just python script.py as per usual)
  2. I would have to figure out how to start & stop the ptb application properly in such a setup …

Docs build it available here.

Copy link
Member

@lemontree210 lemontree210 left a comment

Choose a reason for hiding this comment

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

This looks so great! We're becoming incredibly developer-friendly 😆
I am only suggesting some changes to the comments and, while we're at it, docstrings.

docs/source/examples.customwebhookbot.rst Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
@lemontree210
Copy link
Member

Yes, a full-fledged Django project would have its own configuration files. This is an interesting question: on the one hand, we have to provide a short example, and that's why we're using django.conf.settings, as per Django docs. On the other hand, though, the reason we are providing examples for other frameworks should be that the user already uses one of them in their code and hence, in case of Django, has all these settings in separate files. This might mean that, for a user that doesn't care which app to use for routing of requests, any framework will do, but a user that already uses Django might not find the Django example useful. Then, instead of one example with some lines changed for the respective framework, this actually should expand into something of a tutorial (if you're using Django, but add these lines to urlpatterns, then add this to your settings file etc.). Which, in turn, sort of subverts the idea of having a simple usage example.

@Bibo-Joshi
Copy link
Member Author

This might mean that, for a user that doesn't care which app to use for routing of requests, any framework will do, but a user that already uses Django might not find the Django example useful.

This is a very valid argument, but at least for now I would give more weight to your last point

Which, in turn, sort of subverts the idea of having a simple usage example.

I think whenever two frameworks are being used together one will have to find a midway of usability and best practice. This is a first step in this direction. Maybe we can just include a disclaimer, maybe something along the lines

The following examples show how different Python web frameworks can be used alongside with PTB. This can be useful for two use cases:

  1. For extending the functionality of your existing bot to handling updates of external services
  2. For extending the functionality of your exisiting web application to also include chat bot functionality

How the PTB and web framework components of the examples below are viewed surely depends on which use case one has in mind. We are fully aware that a combinations of PTB with web frameworks will always mean finding a trade off between usability and best practices for both PTB and the web framework and these examples are certainly far from optimal solutions. Please understand them as starting points and use your expertice of the web framework of your choosing to build up on them. You are of course also very welcome to help improve these examples!

?

@lemontree210
Copy link
Member

I think whenever two frameworks are being used together one will have to find a midway of usability and best practice. This is a first step in this direction. Maybe we can just include a disclaimer, maybe something along the lines:

Great disclaimer, agreed :)

Copy link
Member

@lemontree210 lemontree210 left a comment

Choose a reason for hiding this comment

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

Great, just a couple of typos in the disclaimer :)

docs/source/examples.customwebhookbot.rst Outdated Show resolved Hide resolved
docs/source/examples.customwebhookbot.rst Outdated Show resolved Hide resolved
Bibo-Joshi and others added 4 commits August 2, 2023 07:25
Co-authored-by: Dmitry K <58207913+lemontree210@users.noreply.github.com>
Co-authored-by: Dmitry K <58207913+lemontree210@users.noreply.github.com>
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

Wow, didn't realize how similar these examples would be! Unfortuantely I can't setup a webhook right now so can't run these examples. I assume you tested all of them? I also like the inline tabs idea, that's neat

Review comments below are mostly stylistic/design based.

examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_django.py Outdated Show resolved Hide resolved
examples/customwebhookbot_flask.py Outdated Show resolved Hide resolved
@harshil21
Copy link
Member

I assume you tested all of them?

?

@Bibo-Joshi
Copy link
Member Author

@harshil21 Yes

@Bibo-Joshi Bibo-Joshi merged commit 03f8775 into master Aug 16, 2023
24 checks passed
@Bibo-Joshi Bibo-Joshi deleted the doc-webhook-examples branch August 16, 2023 19:15
@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide the customwebhookbot.py example with different frameworks
3 participants