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

Rewrite databases page #1921

Open
Kludex opened this issue Oct 25, 2022 · 14 comments
Open

Rewrite databases page #1921

Kludex opened this issue Oct 25, 2022 · 14 comments
Assignees
Labels
documentation Project documentation good first issue Good for beginners
Milestone

Comments

@Kludex
Copy link
Member

Kludex commented Oct 25, 2022

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 Kludex added documentation Project documentation good first issue Good for beginners labels Oct 25, 2022
@tomchristie
Copy link
Member

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

@gnat
Copy link

gnat commented Oct 28, 2022

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
Copy link

gnat commented Oct 28, 2022

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

@onerandomusername
Copy link
Contributor

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

@tomchristie
Copy link
Member

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

@ni-aas
Copy link

ni-aas commented Nov 12, 2022

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).

@gnat
Copy link

gnat commented Nov 12, 2022

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.

@tomchristie
Copy link
Member

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

Yes.

@i-Ching
Copy link
Contributor

i-Ching commented Dec 8, 2022

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 .

@aminalaee
Copy link
Member

aminalaee commented Mar 24, 2023

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.

@Kludex
Copy link
Member Author

Kludex commented Dec 24, 2023

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
Copy link
Member Author

Kludex commented Dec 24, 2023

Are you still interested in working on this @aminalaee ?

@Kludex Kludex added this to the Version 1.0 milestone Dec 24, 2023
@aminalaee
Copy link
Member

Yeah sure, I can give it a try.

@Kludex
Copy link
Member Author

Kludex commented Feb 12, 2024

Whoever wants to work on this, go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Project documentation good first issue Good for beginners
Projects
None yet
Development

No branches or pull requests

7 participants