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

Blueprint view function name should not contain dots #2790

Closed
gdude2002 opened this issue May 17, 2018 · 2 comments
Closed

Blueprint view function name should not contain dots #2790

gdude2002 opened this issue May 17, 2018 · 2 comments

Comments

@gdude2002
Copy link

gdude2002 commented May 17, 2018

I've done some homework and read up on #2450. I don't really understand the rationale behind that PR, and I don't think it really helps with Flask usage overall. To explain a bit...

I maintain a relatively complex project that builds its own API on top of Flask. This change heavily breaks our application structure, as we currently use dots as separators for view classes under our blueprints. Consider the following:

  • You have sets of blueprints - we use one for each subdomain
  • Each blueprint has a name - for example, main
  • Under a blueprint might be a logical section - for example, there might be an "about" section, so let's call this main.about
  • The logical section will have pages under it - to me, the logical thing to do is use more dots as separators, so main.about.privacy for example

Now, one solution would be to use some other separator. Why not about/privacy? about-privacy? The thing is, regardless of what separator you decide to use there, blueprints must always be separated from the view name using a .. So in, url_for() for example, we would have to use url_for("main.about/privacy"), which just looks plain ugly.

Some sample code A sample route:
from pysite.base_route import TemplateView

class PrivacyView(TemplateView):
    path = "/about/privacy"
    name = "about.privacy"
    template = "main/about/privacy.html"

How we register this route:

class TemplateView(RouteView):
    name = None  # type: str
    path = None  # type: str

    # We have a bunch of inheritance, but here's the relevant method from the superclass.
    # RouteView inherits from MethodView and we dynamically load all our routes on startup.

    @classmethod
    def setup(cls: "RouteView", manager: "pysite.route_manager.RouteManager", blueprint: Blueprint):
        """
        Set up the view by adding it to the blueprint passed in - this will also deal with multiple inheritance by
        calling `super().setup()` as appropriate.

        This is for a standard route view. Nothing special here.

        :param manager: Instance of the current RouteManager
        :param blueprint: Current Flask blueprint to register this route to
        """

        # ... misc code

        blueprint.add_url_rule(cls.path, view_func=cls.as_view(cls.name))

This seems like the correct way to be doing this, right?


So, to summarize, here's what I'm asking:

  1. Why exactly was this change made in the first place?
  2. What should we be doing otherwise?
@davidism
Copy link
Member

davidism commented May 24, 2018

looks plain ugly

Looks fine to me, although personally I'd go with -.

The linked issue explains what's wrong with dots in endpoint names: you can't tell which part is the blueprint vs the endpoint. Dots are for separating blueprints and endpoints. When we implement nestable blueprints (#593, #1548) it would be even more important that dots only separate full names (and would also probably make what you're doing now easier).

For now, the easiest solution is to refactor the names to use another separator. Sorry for the trouble caused!

@gdude2002
Copy link
Author

Well if you don't have nestable blueprints, then the part before the first . is the blueprint, isn't it? Unless you're not using blueprints at all, I suppose.

- does look marginally better I think, yeah. Still not a great situation for us, since we'll have to do a huge amount of replacing, but I guess that can't be helped.

Thanks for the response anyway.

postgres-mirror pushed a commit to pgadmin-org/pgadmin4 that referenced this issue Jun 18, 2018
during generting the routes for the node.

References:
pallets/flask@2f57a0b
pallets/flask#2790

Fixes #3360
Reported by: Marcelo Mendes
Investigated by: Khushboo Vashi
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants