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

Simple route and view config decorator #2852

Open
miohtama opened this issue Dec 7, 2016 · 17 comments
Open

Simple route and view config decorator #2852

miohtama opened this issue Dec 7, 2016 · 17 comments

Comments

@miohtama
Copy link
Contributor

miohtama commented Dec 7, 2016

I am opening this item here for discussion.

When building small scale websites, most views have only one route to them in the form of simple path. Having a separate call for @view_config or add_view() and then add_route() is often unnecessary.

These could be combined to one @route_and_view_config decorator.

Example views.py:

    @routed_view("/", route_name="home")
    def home(request: Request):
        """Our little webby site landing page"""
        content = """
                  (__)                     )__(                vv    vv
                  (oo)                     (oo)                ||----||  *
           /-------\/               *-------\/                 ||     | /
          / |     ||               / |     ||                 /\-------/
         *  ||----||              /  ||----||                (oo)
            ^^    ^^                 vv    vv                (~~)

         American Cow              Polish Cow            Australian Cow
        """
        return Response(content_type="text/plain", body=textwrap.dedent(content), charset="utf-8")

    @routed_view("/awesome", renderer="funky.pt")
    def awesome(request: Request):
        # view_name automatically set to awesome
        # route_name automatically set to awesome

Then in __init__:

from . import views

config.scan(views)  # Will add route and views

Motivation

  • All view related configuration in one place, the decorator, instead of being split between @view_config in views.py and add_route in __init__.py

  • Make development and maintenance easier, especially for newcomers who can get away with less lines and less questions

  • Feature parity with Flask

Existing implementations

Resolving issue between route priorization

In earlier IRC coffee table discussion people have noted that there should be some sort of weight (integer) added to routes, so that routes scanned in different order can be resolved in correct order. Is see this as unnecessary complication to be implemented, but instead suggest to resolve this through documentation. Only use @route_and_view for simple cases. If there are competing routes and view configurations then it is suggested to use calls to add_route and add_view explicitly instead of @route_and_view approach.

Suggested signature

def routed_view_config(func,
                       pattern,
                       route_name=None,
                       view_name=None,
                       factory=None,
                       renderer=None,
                       **kwargs):
    """ A function :term:`decorator` allowing a
    developer to create simple view and corresponding routing configuration.

    This is a :term:`configuration decoration` that
    wraps a :term:`view callable` function and configures
    view and routing arguments for it. The module providing the view
    function must be scanned via :py:meth:`pyramid.config.Configurator.scan`.

    This decorator is intended to be used with
    :ref:`URL dispatch views <_urldispatch_chapter>`
    only. For traversable views you should use
    :py:func:`pyramid.view.view_config`.

    **Minimal usage example**

    For example, this code could in a module ``myapp.views.py``:

    .. code-block:: python

      from pyramid.view import routed_view_config


      @routed_view_config('/')
      def home(request):
          return 'OK'

    Then in ``__init__.py`` or similar place where you set up your
    Pyramid application you need to:

    .. code-block:: python

        config.scan('myapp.views')

    **Rendering views with templates**

    Pass ``renderer``:

    .. code-block:: python

      from pyramid.view import routed_view_config

      @routed_view_config('/', renderer="info.pt")
      def info(request):
          return {"project_name": "Foobar"}

    .. warning::

        ``routed_view_config`` does not handle multiple routes and
        multiple views. An exception is raised in the case there
        are conflicting routes.

    .. seealso::

        :py:meth:`pyramid.config.Configurator.add_route`

        :py:meth:`pyramid.config.Configurator.add_view`

        :py:func:`pyramid.view.view_config`

    Below is a description of most commonly used arguments. Any other arguments through ``kwargs``
    pattern are passed to :py:meth:`pyramid.config.Configurator.add_view`.

    :param pattern:
        Required. Equals to ``pattern`` in :py:meth:`pyramid.config.Configurator.add_route`.
        This can be a simple path like ``/my-view`` or one with the items like ``/my-location/{item}``.
        For item usage see :ref:`matchdict`.

    :param view_name:
        Optional. If ``view_name`` is not given the function name is used.
        Equals to ``name`` in :py:meth:`pyramid.config.Configurator.add_view`

    :param route_name:
        Optional. If ``route_name`` is not given the function name is used.
        Equals to ``name`` in :py:meth:`pyramid.config.Configurator.add_route`.

    :param renderer:
        Optional. View renderer. Usually a template name.
        Equals to ``add_view`` in :py:meth:`pyramid.config.Configurator.add_view`.

    :param factory:
        Route factory.
        Optional. Equals to ``factory`` in :py:meth:`pyramid.config.Configurator.add_route`.

    :param decorator:
        View decorator.
        Optional. Equals to ``decorator`` in :py:meth:`pyramid.config.Configurator.add_view`

    :param permission:
        Permission name required to access the view.
        Optional. Equals to ``permission`` in :py:meth:`pyramid.config.Configurator.add_view`.

    :raise ConflictingRouteConfiguration:
        In the case there is already a route registered with a same name.

    """
    pass
@zupo
Copy link
Contributor

zupo commented Dec 7, 2016

I would very much like to see something like this in core and I pledge to help write code, tests and/or docs. Would make teaching/demoing Pyramid to newcomers simpler.

@JanLikar
Copy link
Contributor

JanLikar commented Dec 7, 2016

This is great and I think it would significantly decrease the barrier of entry.

@JanLikar
Copy link
Contributor

JanLikar commented Dec 7, 2016

I recommend a different name, though. I think @route_and_view looks a bit odd, I would much prefer to use @routed_view. I find it more descriptive and clean looking.

@miohtama
Copy link
Contributor Author

miohtama commented Dec 7, 2016

@JanLikar Updated the propsosal

@blaflamme
Copy link
Member

I like @routed_view or @routed_view_config if we want to keep the *_config suffix.

@mmerickel
Copy link
Member

It'd help me a lot to talk at least a little bit about what an "ideal" solution is and see how far we are from that before resolving to settle on resolving issues through documentation. Obviously we have pushed hard against such a feature in Pyramid for a long long time (ever since I've been around, which is 1.0 days) and I haven't seen many proposals on how to fix it - just a lot of requests to make things more like flask. They basically all cut corners, which is fine until it goes into core.

Without the weighting it will likely be inferior by design versus flask's solution which uses a typed routing syntax where each type is assigned weights. It should be obvious that this is super important for any route that has a pattern.

Another thing to note is that Pyramid already did this once where config.add_route(...) accepted some view parameters in order to combine the two. We ended up removing it (in Pyramid 1.1) for a couple reasons.

a) It was confusing which parameters were for the view and which were for the route. Some weren't possible. The old approach was config.add_route('home', '/', view_renderer='string', ...).

b) A lot of newcomers who saw that code didn't realize you could have more than one view for each route with different predicates (a distinguishing feature of Pyramid).

This proposal fixes things partially by namespacing the route options into route_config_args={...} which certainly helps but also makes those arguments feel like second-class citizens. For example @routed_view_config('/', route_name='home', route_config_args=dict(factory=HomeFactory)). This seems worse to me.

@miohtama
Copy link
Contributor Author

miohtama commented Dec 8, 2016

@mmerickel Let me reiterate that this is a solution for very simple routing cases, not all of them.

  • We want to make a shortcut for writing simple views simple

I am afraid we cannot reach ideal solution here that would solve all the issues, as Pyramid has the most complex routing logic of all the web frameworks out there. For complex use cases it is still better to use add_route and view_config. Especially we are not concerned about the cases that views can have multiple routes and vice versa.

For example like use case:

  @routed_view_config('/', route_name='home', route_config_args=dict(factory=HomeFactory))

Here factory is set. By the problem definition, this might be considered one of more complex use cases.

To reach a community consensus here, I suggest we now look through our Pyramid project source codes for different route + view config examples and list ones we'd like to cover with routed_view.

@jean
Copy link

jean commented Dec 8, 2016

this is a solution for very simple routing cases

To resolve ambiguity, raise an error during scan if @routed_view decorators end up registering views on the same route and point at the standard way.

@mfrlin
Copy link
Contributor

mfrlin commented Dec 8, 2016

If @routed_view_config is intended to simplify things I think route_config_args=dict(factory=HomeFactory) is not a good idea as it is confusing for beginners. Maybe limit what @routed_view_config can do by just not accepting certain parameters and the ones it has could be prefixed with route_ and view_ depends if they come from add_route or add_view?

@miohtama
Copy link
Contributor Author

miohtama commented Dec 8, 2016

Based on the feedback from @jean and @mfrlin, I have updated the proposal.

Based on grepping my various Pyramid based projects using @simple_route, I picked most common arguments that I have been using and documented them.

I want the signature to be more static, so that newcomers find it easier and IDEs provide autocompletion.

@zupo
Copy link
Contributor

zupo commented Dec 8, 2016

The updated signature looks OK to me.

@mmerickel
Copy link
Member

@jean

To resolve ambiguity, raise an error during scan if @routed_view decorators end up registering views on the same route and point at the standard way.

This is not an easy thing to do at all. It's hard to put any weight on statements like this when no proof of concept has been written on accomplishing the comparison of arbitrary regexes. It sounds great when you say it, but that's about as far as it is helpful.

@miohtama

Let me reiterate that this is a solution for very simple routing cases, not all of them.

I feel like you might be conveniently ignoring that we already had a solution for "simple routing cases" that we removed for real reasons dealing with confusion. What exactly makes this proposal better?

One of your motivations above is "feature parity with flask". This proposed API is absolutely not that. Can we agree on that? It is in no way usable in the variety of situations that flask's route decorator is usable. I'd really appreciate if some of the other COMMON issues we see with routing were taken into account when proposing a new API, instead of going for the lowest common denominator. For example, fitting routes/views into the security system is by far the #1 question I see about routes on IRC. Another is how to improve REST APIs, and instead this is focused on simple views that handle rendering a template.

@miohtama
Copy link
Contributor Author

miohtama commented Dec 9, 2016

@mmerickel The current Pyramid solution for a view registration is not simple as you need to update two files for adding one view: __init__.py with add_route() call and then @view_config in views.py. Because we could get away with one line of view registration and route registration. This is what I feel people from the community want as it has been discussed here. This kind of feature is useful and thus tomb_routes and simple_route were born.

Do we agree on this or not, as we need to find some common ground to have a starting point? If we do agree on this that there is need for such thing then we can take one step backward and start discussing how we can work towards having this in Pyramid core.

@jean
Copy link

jean commented Dec 9, 2016

@mmerickel

accomplishing the comparison of arbitrary regexes

That would be very hard indeed, but I don't seen any arbitrary regexes, only a simple path like /my-view or one with the items like /my-location/{item}.

A routed_view could clash with views registered the usual way, but views registered the usual way can clash with each other too, and if you're using them, you should be prepared to debug them.

@miohtama would routed_views take precedence?

@miohtama
Copy link
Contributor Author

miohtama commented Dec 9, 2016

In the end it goes like this

  1. There is a decorator with some parameters (a)
  2. There is config.scan()
  3. Mystery (b)
  4. There is a call to add_view (same as @view_config) and add_route

We are now discussing (a) and (b). In the end, it doesn't change the underlying logic of Pyramid. It only makes one manual call to add_route to go away. So my take here is that it will respect the underlying Pyramid routing configuration, but only in more convenient form. If we need to add some extra features to cover corner cases we can iterate through them one by one in a checklist.

@mmerickel
Copy link
Member

@miohtama

Do we agree on this or not, as we need to find some common ground to have a starting point? If we do agree on this that there is need for such thing then we can take one step backward and start discussing how we can work towards having this in Pyramid core.

Definitely - I wouldn't go through all the effort of analyzing the API if I didn't think it'd be a great feature to add to Pyramid. I've put a lot [1] of thought into how we can even make Pyramid extensible enough to experiment with various ways of solving this. However adding it to core comes at a cost, and I have pretty high standards for what the feature needs to be in order to avoid history repeating itself.

I can't justify adding a decorator that is by design inferior to flask's. This exact feature and example that you're proposing is in our core design defense [2] as a Bad Idea (tm). If we can't improve on it to make that design defense invalid, then how can I accept it? Solving this with documentation just isn't good enough to me.

[1] http://docs.pylonsproject.org/projects/pyramid/en/latest/narr/extconfig.html#calling-actions-from-actions
[2] http://docs.pylonsproject.org/projects/pyramid/en/latest/designdefense.html#routes-need-relative-ordering

@merwok
Copy link
Contributor

merwok commented Feb 9, 2020

The current Pyramid solution for a view registration is not simple as you need to update two files for adding one view: __init__.py with add_route() call and then @view_config in views.py.

This is only one way of doing it, but nothing in Pyramid forces it.
The official docs start with a one-file app!

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

No branches or pull requests

8 participants