starlette icon indicating copy to clipboard operation
starlette copied to clipboard

Rewrite databases page

Open Kludex opened this issue 3 years ago • 16 comments

The idea is to rewrite the databases page on our documentation: https://www.starlette.io/database/ .

Instead of recommending databases, we should recommend pure SQLAlchemy.

cc @zzzeek (just pinging to let you know about our intentions, no need to react)

[!IMPORTANT]

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

Kludex avatar Oct 25 '22 09:10 Kludex

The databases package is also used in the config.md example. We should remove it from there too.

I can't see it used anywhere else. https://github.com/encode/starlette/search?q=databases

lovelydinosaur avatar Oct 25 '22 09:10 lovelydinosaur

Ah that's unfortunate. Databases was a great concept- SQLAlchemy's documentation is still some of the most verbose and troublesome to navigate in the Python community, and I don't see that being addressed anytime soon- even if the new 1.4 (and 2.0 API) is a good option once you learn it and can treat the SQLAlchemy docs strictly as a reference manual rather than something one reads top to bottom.

gnat avatar Oct 28 '22 19:10 gnat

To your credit, @tomchristie all your documentation can be read from top to bottom, and that's underappreciated and lovely.

gnat avatar Oct 28 '22 19:10 gnat

Does this mean that databases is deprecated or will be soon?

onerandomusername avatar Nov 02 '22 08:11 onerandomusername

Thank you for your comment, @gnat. Much appreciated. Let's treat this as a proposal, rather than as a given.

lovelydinosaur avatar Nov 03 '22 11:11 lovelydinosaur

I see SQLAlchemy has AsyncSession and create_async_engine, etc. Is the reason behind databases that there was no async capabilities for it at the time? Is it possible to use it directly without databases now? (Sry for noob question).

ni-aas avatar Nov 12 '22 11:11 ni-aas

I imagine if SQLAlchemy had asyncio previously, databases would likely have never been created, even with the poor developer docs story.

https://docs.sqlalchemy.org/en/20/changelog/migration_14.html#change-3414

tl;dr: SQLAlchemy makes use of greenlet to support asyncio.

I'm not terribly familiar with the greenlet library, so if anyone can chime in with real world experiences, that would be fantastic.

gnat avatar Nov 12 '22 12:11 gnat

Is the reason behind databases that there was no async capabilities for it at the time?

Yes.

lovelydinosaur avatar Nov 14 '22 12:11 lovelydinosaur

To Starlette team:

  1. Please consider to provide multiple databases examples (vertical partitioning) of SQLAlchemy into your coming documentation. https://docs.sqlalchemy.org/en/14/orm/persistence_techniques.html#session-partitioning

I find that features of SQLAlchemy providing more consideration for real life. (For another example: horizontal partitioning https://docs.sqlalchemy.org/en/14/orm/examples.html#examples-sharding)

I follow the above url of SQLAlchemy document that I can make a starlette program with multiple databases support. The implementation steps are: 1.1 one database to one engine 1.2 one engine has many tables 1.3 the key point is "binds of Session" Session.configure(binds={table1: engine1, table2: engine2, table3: engine2}) Reference: https://stackoverflow.com/questions/28238785/sqlalchemy-python-multiple-databases

  1. It is right for Kludex. His first comment above: "we should recommend pure SQLAlchemy".

  2. flask-sqlalchemy has not yet completed this feature in present version, but it quotes coming next: Deprecated since version 3.0: Will be removed in Flask-SQLAlchemy 3.1. db.session supports multiple binds directly. Reference: https://flask-sqlalchemy.palletsprojects.com/en/3.0.x/api/#flask_sqlalchemy.SQLAlchemy.Model

  3. Django has documented its multiple database support, sorry I don't test Django because I don't have time to study Django Reference: https://docs.djangoproject.com/en/4.1/topics/db/multi-db/

We are engineers, we know the good quality of code in Starlette. But managers, decision makers, or my bosses like to read the title of document like above Django link

In summary, I suggest your coming document to target more non-technical outsiders to know the strength of enterprise-ready Starlette .

i-Ching avatar Dec 08 '22 21:12 i-Ching

I've been thinking to document the SQLAlchemy docs with latest SQLAlchemy v2 API, which is a bit different from V1, I'm open to suggestions but I think doing it for v2 will avoid a redo in the future and is much nicer.

aminalaee avatar Mar 24 '23 07:03 aminalaee

For the database.md, there's a lot to remove/change.

For the person that will work on this, I've created this example to replace the one we have in that page:

from contextlib import asynccontextmanager
from typing import AsyncIterator

from sqlalchemy import select
from sqlalchemy.ext.asyncio import AsyncAttrs, async_sessionmaker, create_async_engine
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column

from starlette.applications import Starlette
from starlette.config import Config
from starlette.requests import Request
from starlette.responses import JSONResponse
from starlette.routing import Route

# Configuration from environment variables or '.env' file.
config = Config(".env")
DATABASE_URL = config("DATABASE_URL")

# SQLAlchemy setup
engine = create_async_engine(DATABASE_URL, echo=True)
async_session = async_sessionmaker(engine, expire_on_commit=False)


class Base(AsyncAttrs, DeclarativeBase):
    pass


class Note(Base):
    __tablename__ = "notes"

    id: Mapped[int] = mapped_column(primary_key=True)
    text: Mapped[str]
    completed: Mapped[bool]


# Main application code
@asynccontextmanager
async def lifespan(app: Starlette) -> AsyncIterator[None]:
    # Create tables
    async with engine.begin() as conn:
        await conn.run_sync(Base.metadata.create_all)
    yield
    await engine.dispose()


async def list_notes(request: Request):
    async with async_session() as session:
        query = await session.execute(select(Note))
        results = query.scalars().all()

    return JSONResponse(
        [{"text": result.text, "completed": result.completed} for result in results]
    )


async def add_note(request: Request):
    data = await request.json()
    new_note = Note(text=data["text"], completed=data["completed"])

    async with async_session() as session:
        async with session.begin():
            session.add(new_note)

    return JSONResponse({"text": new_note.text, "completed": new_note.completed})


routes = [
    Route("/notes", endpoint=list_notes, methods=["GET"]),
    Route("/notes", endpoint=add_note, methods=["POST"]),
]

app = Starlette(routes=routes, lifespan=lifespan)

Kludex avatar Dec 24 '23 10:12 Kludex

Are you still interested in working on this @aminalaee ?

Kludex avatar Dec 24 '23 10:12 Kludex

Yeah sure, I can give it a try.

aminalaee avatar Dec 24 '23 20:12 aminalaee

Whoever wants to work on this, go ahead.

Kludex avatar Feb 12 '24 21:02 Kludex