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

Version 0.13 #704

Merged
merged 11 commits into from Nov 13, 2019
Merged

Version 0.13 #704

merged 11 commits into from Nov 13, 2019

Conversation

@tomchristie
Copy link
Member

tomchristie commented Nov 5, 2019

Documentation still in progress.

This PR does not significantly remove any public API, but it does start to move us to a new prefered style of routing, moving away from the decorator style.

I think using plain route lists is more clearly decoupled, and gives a clearer indication of what's going on under the hood.

For example...

routes = [
    Route('/', homepage),
    Mount('/users', routes=[
        Route('/', users, methods=['GET', 'POST']),
        Route('/{username}', user),
    ])
]

app = Starlette(routes=routes)

Similarly, middleware now has a prefered configure-on-init style...

routes = ...

middleware = [
    Middleware(SentryAsgiMiddleware) if settings.SENTRY_DSN else None,
    Middleware(HTTPSRedirectMiddleware) if settings.HTTPS_ONLY else None,
    Middleware(SessionMiddleware, secret_key=settings.SECRET, https_only=settings.HTTPS_ONLY)
]

app = Starlette(routes=routes, middleware=middleware)

Still to do here...

  • Review exception_handlers / server_error. - We should defer this to a future release.
  • Update starlette-example.
  • Update Routing docs.
  • Update Middleware docs.
  • Update routing examples in docs throughout.

We may also want to reivew our url_for and url_path_for documentation and API as part of this release version. I'm somewhat keen for us to switch things around there, so that path-only URLs are the default style, and there's less surface area for folks to need to care about configuring Uvicorn's --allow-forward-ips. For example if we were using this:

  • app.url_for(), request.url_for(), url_for() in templates. Return a relative path-only URL.
  • request.absolute_url_for(), absolute_url_for() in templates. Return a fully qualified URL.

I'm also pretty keen to switch things up here so that when debug=True we...

  • Display helpful routing table info if no route matches during routing.
  • Display helpful routing table info when url_for(...) fails.

Something else that I think is interesting here is that we're more clearly exposing that alternative subclasses of BaseRoute can also be added to the routing table. For example, it'd be interesting to consider if there's any worthwhile documentation work to do in the ecosystem around eg. noting how you can use FastAPI's APIRoute inside a regular Starlette application, to point at a handler which gets their dependency injection treatments, alongside other regular Route cases, which just get the standard request argument.

That line of thought is as much about demystifying how the different bits fit together, about making sure that we can display useful routing debug info for higher level frameworks as well as the plain starlette case, and also about enabling other framework authors to take the happy path when building on top of Starlette. (We kinda missed a trick with Responder here, which functions differently enough that it's working against the grain.)

tomchristie added 2 commits Nov 5, 2019
@florimondmanca

This comment has been minimized.

Copy link
Contributor

florimondmanca commented Nov 5, 2019

Interesting take! I think it gets Starlette closer to a declarative style, which for end users is almost always preferable.

So as I understand, the current imperative API (.add_route() etc) will stay in place? I think that staying public and a first-class citizen (though not recommended for "Starlette-native" apps) is important for integrators and people building on top of Starlette.

Just a small note — in most cases Black is probably going to reformat these big lists a bit differently, e.g.:

routes = [
    Route("/", homepage),
    Mount(
        "/users",
        routes=[
            Route("/", users, methods=["GET", "POST"]),
            Route("/{username}", user),
        ],
    ),
]

I think it looks OK, but just wanted to get this out there. When I first saw the Channels documentation with those huge lists made out of strangely-named objects, it didn't look very welcoming to be as a beginner. Luckily, names of Starlette's components are fairly short and explicit, so that shouldn't be a problem here. People can still break things down, e.g.

users_routes = [
    Route("/", users, methods=["GET", "POST"]),
    Route("/{username}", user),
]

routes = [
    Route("/", homepage),
    Mount("/users", users_routes),
]

In larger projects, they'll most likely import these routes from a different module anyway.

@tomchristie

This comment has been minimized.

Copy link
Member Author

tomchristie commented Nov 5, 2019

Can you confirm the current imperative API (.add_route() etc) will stay in place?

Yes, that's not going away here. However I do think that I'm going to stop documenting it everywhere.

Would I like to eventually phase it out? Possibly at some point in the future, maybe. One of the key things I'm trying to emphasize in Starlette is a low-complexity stack, and components such as the Starlette and Router classes get really simple if we move away from all the .add_route(...) and @route() futzing.

Just a small note — in most cases Black is probably going to reformat these big lists a bit differently

Indeed, yes. I'm actually not super wild about that.

The hostedapi project routing looks like this at the moment...

routes = [
    Route("/", endpoints.dashboard, name="dashboard", methods=["GET", "POST"]),
    Route("/tables/{table_id}", endpoints.table, methods=["GET", "POST"], name="table"),
    Route(
        "/tables/{table_id}/columns",
        endpoints.columns,
        methods=["GET", "POST"],
        name="columns",
    ),
    Route(
        "/tables/{table_id}/delete",
        endpoints.delete_table,
        methods=["POST"],
        name="delete-table",
    ),
    Route(
        "/tables/{table_id}/upload", endpoints.upload, methods=["POST"], name="upload"
    ),
    Route(
        "/tables/{table_id}/columns/{column_id}/delete",
        endpoints.delete_column,
        methods=["POST"],
        name="delete-column",
    ),
    Route(
        "/tables/{table_id}/{row_uuid}",
        endpoints.detail,
        methods=["GET", "POST"],
        name="detail",
    ),
    Route(
        "/tables/{table_id}/{row_uuid}/delete",
        endpoints.delete_row,
        methods=["POST"],
        name="delete-row",
    ),
    Route("/500", endpoints.error),
    Mount("/static", statics, name="static"),
]

I guess we could suggest that folks use # fmt: off, for a single route-per-line style...

# fmt: off
routes = [
    Route("/", endpoints.dashboard, name="dashboard", methods=["GET", "POST"]),
    Route("/tables/{table_id}", endpoints.table, name="table", methods=["GET", "POST"]),
    Route("/tables/{table_id}/columns", endpoints.columns, name="columns", methods=["GET", "POST"]),
    Route("/tables/{table_id}/delete", endpoints.delete_table, name="delete-table", methods=["POST"]),
    Route("/tables/{table_id}/upload", endpoints.upload, name="upload", methods=["POST"]),
    Route("/tables/{table_id}/columns/{column_id}/delete", endpoints.delete_column, name="delete-column", methods=["POST"]),
    Route("/tables/{table_id}/{row_uuid}", endpoints.detail, name="detail", methods=["GET", "POST"]),
    Route("/tables/{table_id}/{row_uuid}/delete", endpoints.delete_row, name="delete-row", methods=["POST"]),
    Route("/500", endpoints.error),
    Mount("/static", statics, name="static"),
]
# fmt: on

That's still a bit heavy, so I'm open to conversations there.

I do think this is a bit of an awkward point because I think the style expresses the internals as clearly as possible, but it's not neccessarily how I'd want it to display.

Also worth noting that it's much nicer for allowing us to do expressive, complex stuff such as...

routes = [
    Host("api.example.com", name="api", routes=[
        Route("/users/", user_list, name="user-list"),
        Route("/users/{username}", user_detail, name="user-detail"),
    ]),
    Host("www.example.com", routes=[
        Route("/", homepage, name="homepage"),
        Mount("/auth", routes=auth.routes, name="auth"),
        Mount("/statics", app=statics, name="static"),
    ])
]
@florimondmanca

This comment has been minimized.

Copy link
Contributor

florimondmanca commented Nov 5, 2019

Yes, the big list style has value for sure. When I think about it, in the world of frontend frameworks most routing is defined this way, declaratively, using either an array or an object (at least, Angular and Vue Router work like this). We haven’t been particularly used to it in the Python backend side, apart from a lighter form of it in Django, but I really like the simplicity behind it.

Also, I didn’t know Starlette supported subdomain routing via Host! That’s a really handy feature.

@tomchristie

This comment has been minimized.

Copy link
Member Author

tomchristie commented Nov 5, 2019

Also, I didn’t know Starlette supported subdomain routing via Host! That’s a really handy feature.

Indeed - it's not actually documented yet (and I need to add support for routes=).

In practice it's actually a bit awkward to use in local development, so may need to put up a bit of documentation on how to use tools such as ngrok, or how to edit /etc/hosts or something.

@sandys

This comment has been minimized.

Copy link

sandys commented Nov 6, 2019

the routing change makes me ask - do you have any plans for a starlette project generator ? like a "rails new" command. the routing config could be put in a separate file (pretty much like routes.rb) and turn off formatting for black as well.

As more and more declarative features get included - a project generator would be very very useful.

@tomchristie

This comment has been minimized.

Copy link
Member Author

tomchristie commented Nov 6, 2019

@sandys I'm not exactly sure what we'll want to do there.

It's possibly that we want to leave that kind of thing for higher level frameworks like Fast API to explore.

Or, it might be that we just recommend something along these lines...

$ git clone https://github.com/encode/starlette-example myproject
$ cd myproject
$ rm -r .git

(Note that the starlette-example repo needs updating in line with the 0.13 changes)

Or it could be that, yes, it makes sense for use to have a lightweight starlette init command.

@sandys

This comment has been minimized.

Copy link

sandys commented Nov 6, 2019

ideally a starlette init would be very useful (i daresay far more useful than the documentation in guiding what is best practice and what is not!)

the alternative is the example project. Actually we are treating hostedapi as pretty much that !

@florimondmanca

This comment has been minimized.

Copy link
Contributor

florimondmanca commented Nov 6, 2019

From past experience, project code generation is not necessarily the best approach when it comes to hinting at best practices.

Sure, it might be convenient, but it can turn out to be hard to maintain, and IMO it prevents users from actually thinking about how they'd like to set things up. It might make sense for super-opinionated frameworks like Django, but I don't think it'd bring much value to a super lightweight one like Starlette.

I think ideally the documentation should serve as an example, and point to example projects (starlette-example/HostedAPI) if users want to see code of real-world-like apps.

Edit: perhaps let's open an issue for this particular topic if there's more discussion needed?

@sandys

This comment has been minimized.

Copy link

sandys commented Nov 6, 2019

i dont agree with the forced usage of "users should think" and "lightweight frameworks should not have generators"

I think a framework can be lightweight and yet handholds users. A great example of this is alembic.
Alembic provides init and revision which helps setup the environment and then create a migration each time. It takes care of checks and checksums, etc

Obviously its @tomchristie 's prerogative here ...but there are sufficient examples of fantastic frameworks that do this.

@gvbgduh

This comment has been minimized.

Copy link
Member

gvbgduh commented Nov 6, 2019

I think it might have sense what @sandys proposes.

The beauty of starlette that it fits many purposes as it's also a perfect level for big or custom projects, it's not that high level to customize existing functionality, so you can have full control of what you do and it's not too low level to have unreasonable boilerplate.

In case the goal is simple and a lightweight app it might end up as everything in one file and in a few, but you generally wouldn't be thinking about the project structure and its template.

Where in case of a bigger project it might be handy to have some guide around (like Django) does. It's also the case where the route decorators become a mess as spread through the whole project. flask-RESTful also sort or guides towards this direction.

But be that as it may, It's tricky indeed to come up with such a template that would fit custom purpose projects and higher-level frameworks as well.

@euri10

This comment has been minimized.

Copy link

euri10 commented Nov 6, 2019

From past experience, project code generation is not necessarily the best approach when it comes to hinting at best practices.

Sure, it might be convenient, but it can turn out to be hard to maintain, and IMO it prevents users from actually thinking about how they'd like to set things up. It might make sense for super-opinionated frameworks like Django, but I don't think it'd bring much value to a super lightweight one like Starlette.

+1 on this, I not only fully agree but this is proved on a daily basis, just take a look at issues on https://github.com/tiangolo/full-stack-fastapi-postgresql/issues

Most questions have nothing to do with the framework: how I add a celery tasks, my traefik container doesn't work, how do I connect to pgadmin, how can I add a user to the db, how can I debug in docker ?

The result is simple, issues are not addressed.
It effectively makes people lazy and fail to search and think.

@sandys

This comment has been minimized.

Copy link

sandys commented Nov 6, 2019

@tomchristie

This comment has been minimized.

Copy link
Member Author

tomchristie commented Nov 6, 2019

Let's move any convo on init and layouts over here... #705

@tiangolo

This comment has been minimized.

Copy link
Member

tiangolo commented Nov 6, 2019

About the list of routes vs decorator-style:

I see how this helps to structure parts by their type, all routing is done in a single place, in a list, and the functions that handle each route are separated in another place.


On the other side, my argument in pro of decorator style is that it ties things together by the feature or subject they are related to.

So, the code that handles a username is near the declaration of the route /user/{username}.

I find that especially useful with routes with path parameters like:

@app.route('/user/{username}')
def user(request):
    username = request.path_params['username']
    return PlainTextResponse('Hello, %s!' % username)

In this case, the username in '/user/{username}' is close to the one in request.path_params['username'], just a few lines above.

If someone was refactoring that to make it user_name instead of username (just a silly example), after changing the path to '/user/{user_name}' it would be easier to remember to also change the parameter extraction to request.path_params['user_name'], just because they are visually closer. And if that person forgot to refactor it inside, finding the bug later would probably be easier still.

Having those two related texts naming something in different files could, in some cases, make that refactor a bit more difficult.

This is similar to the philosophy in React, of having code of different types (HTML-ish JSX and JavaScript) close together, in the same file/component, but related in their subject, not the type of code. And React-router works in a similar way.

I actually didn't like React until recently, that I started using it and it made sense to me (I used only Angular and Vue before that). But it also seems that that idea (React itself) ended up being quite popular.


The preference between these two can quickly become quite subjective and dependent on personal/team taste, so I understand if this is the preferred way for Starlette. I still wanted to share that additional aspect of it, that might be important.

For FastAPI it's quite more relevant, as there's even more "shared information" between the part that declares a route path (the decorator) and the code in the function that handles it, so I'll keep that as the default there.

As long as I can keep using decorators in FastAPI, I'm OK with the decision. It currently uses .add_route(), so I would be inclined towards keeping it.

But even if not, if there's a way to update the routes incrementally (I guess some self.router.routes), that would be fine for me.

@dmontagu

This comment has been minimized.

Copy link
Contributor

dmontagu commented Nov 6, 2019

@tiangolo Yeah, in addition to the obvious awkwardness posed by path parameters, I think the various response_X arguments to the FastAPI decorators are useful, and are best included near the actual endpoint function definition (maybe that's what you meant by "shared information").

If those arguments had to be placed in a routes list rather than next to the endpoint definition, I think it would get hard to maintain.

I have some ideas about how FastAPI might be changed to make it easier to follow this approach, but I'll discuss those in tiangolo/fastapi#687.

@tiangolo

This comment has been minimized.

Copy link
Member

tiangolo commented Nov 6, 2019

Yep, that's what I meant @dmontagu. And, yeah, my plan in FastAPI is to keep decorators.


Still, it shouldn't be a problem, FastAPI doesn't have to use the same style as Starlette, it can use it internally as an ASGI toolkit (one of Starlette's its objectives). As long as Starlette supports it.

Also, Starlette's goals are probably more inclined to be a very lightweight, simple, ASGI toolkit, with a small surface and a clean and simple codebase (it's beautiful inside) that can be easily extended and used by many other tools.

While FastAPI's goals don't include being a toolkit for other frameworks, nor having an as-simple-as-possible codebase. The goals are heavily inclined towards the final API developer's "user/development experience". As an example, all the app HTTP methods like app.get(), app.post() are just syntax sugar, and increase the internal codebase surface/complexity, required tests, etc. Just to improve final developer's experience a bit, reducing their app's codebase surface and complexity. That's also why there are lots and lots of docs, even for some things that are not strictly related to FastAPI itself.

So, it's OK if both projects adopt/suggest different styles in some places, their objectives are probably a bit different.

And also, although decorator vs list styles look quite different, supporting decorator style in FastAPI wouldn't be that difficult while still using Starlette's awesomeness.

Anyway, we can continue the FastAPI specific discussion in that issue as @dmontagu suggests, but I still wanted to bring some attention to that point of the closeness in code of related subjects, in case that's something important for Starlette itself.

@tomchristie

This comment has been minimized.

Copy link
Member Author

tomchristie commented Nov 6, 2019

@tiangolo - Very much agree with all the above, yup! 😃

And yeah, totally dig the "closeness of related concerns". I think the overall trade-off is just a bit different for Starlette, than it is for FastAPI.

tomchristie added 3 commits Nov 7, 2019
tomchristie added 3 commits Nov 13, 2019
@tomchristie tomchristie merged commit 7f8cd04 into master Nov 13, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@tomchristie tomchristie deleted the version-0.13 branch Nov 13, 2019
seanharrison added a commit to BlackEarth/starlette that referenced this pull request Nov 27, 2019
* commit 'b8bd1696492e501bd617bd151278999c68b30e2b': (39 commits)
  Update database.md
  Update routing.md
  Drop double square brackets tests in install script (encode#718)
  Show nicer warning when `ujson` isn't installed (encode#728)
  Tweak app signatures (encode#723)
  Added support for publishing wheel files. Fixes encode#668 (encode#719)
  📝 Fixes example typo in README (encode#717)
  Version 0.13.0
  Version 0.13 (encode#704)
  List Scout APM as a third party package (encode#710)
  Add Authlib into third party packages. (encode#712)
  Fix a small typo in CORS middleware documentation (encode#708)
  Handle trailing slash redirects properly. (encode#707)
  Strip whitespace on response headers (encode#706)
  Update release-notes.md
  Remove Bocadillo from third party packages (encode#684)
  Version 0.12.13
  Support app configuration on initialization. (encode#702)
  Version 0.12.12
  Fix url_for with double mounts (encode#701)
  ...
@luckydonald

This comment has been minimized.

Copy link

luckydonald commented Dec 24, 2019

Coming from Flask instead of Django I really appreciate how you can just open up a view file, and instantly read what's served here.

I was just about to switch to FastAPI, and with it this project.
And even with it being not encouraged to do so, I highly prefer the @app.decorator approach.

It just suits some projects much better, where getting something up fast (but without being sloppy).

That's something I experienced with flask very well:

First you have an idea. A simple web app or API which should deliver things.
You add a main.py add app = Flask(__name__) and are good to go to get a first working prototype set up very quickly.
You don't have to set up routes and edit 20 files *exaggeration just to get a simple hello world running.

And that's a very pythonic way of doing things.
In for example java you have to first build a

class Main {
  public static void main(String args[]) {
    // your code here
  }
}

just to get anything running.
Python is not like that.

So back to our example. The app we're building is now getting more complex and so does our code.
No problem, we split it up in database modules, templates etc.
And our routes get split up in views, by using blueprints.

So that's growing very good with our app, and if it is getting big there is no problem there.

Some apps however never need to grow, they stay simple.
Imagine a simple API, a personal blog, or a one-shot landing page for a product with just a few email newsletter functions.

That's when setting up a dedicated routing file is more of a burden than a feature.
And that's how all of my projects start out, commercially or private: Small.

That's why I love the decorator way, it's simple and I have to edit only one file.

@luckydonald

This comment has been minimized.

Copy link

luckydonald commented Dec 24, 2019

I see why forcing the list style makes sense for more let me call it "prepared" projects, where you have a clear picture from the beginning how your routing will look like, e.g. when you have to conform to a functional specifications document or similar.

However I'd love to be able to use the old style as I'm now starting with a new project and looking for a good async alternative of flask and have also interest in using the promising FastAPI.

I can understand the idea of dropping it from the framework for speed gain and less code, which is a valid goal. Maybe that future plan could instead of dropping it completely just refactor them in a separate project build on Starlette (maybe Starlette-decorators, I'm bad with names, lol), similar of how FastAPI is a good addition?

@luckydonald

This comment has been minimized.

Copy link

luckydonald commented Dec 24, 2019

Also could you maybe serve a pre-0.13 version of the docs, to make it easier for those who still like to use (or have to use) the decorators?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.