-
Notifications
You must be signed in to change notification settings - Fork 4
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
Context manager for startup and shutdown of connection #37
Comments
Actually, you might add this. It shouldn't be problematic to implement. I like the idea, although in docs you should state that people should not call it everytime they want to execute something, because it's very expensive to startup scylla, since it creates the connection pool inside. |
Cool! I will use it with dependency injection in FastAPI; the connection will shut down naturally when we stop the server or if the WSGI dies for some reason. |
Can you provide me with example of how you want to use it? Minimal application. |
This should work (but is wrong, correct approach): app = FastAPI()
async def get_scylla_session() -> AsyncGenerator[Scylla, None]:
with Scylla(...) as scylla:
yield scylla
@app.get("/")
def root_endpoint(scylla: Scylla = Depends(get_scylla_session)):
# scylla can be used to interact with DB
... |
You should not do so under any circumstances. It is going to be disastrously slow, because it is going to create a connection pool on every request. When you call shutdown it closes all connections to scylla. You don't want such thing. So, that's how you actually need to use it: from fastapi import Depends, FastAPI, Request
from typing import Any, AsyncGenerator
from contextlib import asynccontextmanager
from scyllapy import Scylla
@asynccontextmanager
async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
print("Starting up")
scylla = Scylla(
["localhost:9042"],
username="scylla",
password="scylla",
keyspace="test",
)
await scylla.startup()
app.state.scylla = scylla
yield
await scylla.shutdown()
print("Shutting down")
app = FastAPI(lifespan=lifespan)
async def get_scylla_session(request: Request) -> Scylla:
return request.app.state.scylla
@app.get("/")
async def root_endpoint(
scylla: Scylla = Depends(get_scylla_session),
) -> list[dict[str, Any]]:
res = await scylla.execute("SELECT * FROM test")
return res.all() Here we initialize pool only once and then reuse connections. We store the initialized scylla in the state, so we can access it from dependencies and use in our handlers. This setup is going to work many times faster. |
Just tested this, you are correct. After this PR, the code should be like this: from fastapi import Depends, FastAPI, Request
from typing import Any, AsyncGenerator
from contextlib import asynccontextmanager
from scyllapy import Scylla
@asynccontextmanager
async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
print("Starting up")
scylla = Scylla(
["localhost:9042"],
username="scylla",
password="scylla",
keyspace="test",
)
app.state.scylla = scylla
with scylla:
yield
app = FastAPI(lifespan=lifespan)
async def get_scylla_session(request: Request) -> Scylla:
return request.app.state.scylla
@app.get("/")
async def root_endpoint(
scylla: Scylla = Depends(get_scylla_session),
) -> list[dict[str, Any]]:
res = await scylla.execute("SELECT * FROM test")
return res.all() |
After further review of the scyllapy documentation, it's now clearer in terms of how I can be able to work on the keyspaces. The only bit that is not clear is how I can be able to create tables using models i.e. how to implement a models based table definition for the below snippet of code obtained from the scyllapy documentation:
|
I got your emails, was going to answer; however, this is too much. Can you condense your question, and remove it from here? Can we turn on the discussion section for future cases @s3rius ? Update: @Arap-Odawa you can't post this here. Create a new issue. |
Hi @caniko , I have migrated my question to a new issue: https://github.com/Intreecom/scyllapy/issues/42 Thanks and apologies for the earlier many postings, I have cleaned them up. Regards, Odawa |
Looking for this:
__aenter__
should dostartup
and__aexist__
should doshutdown
.If this is OK, I'd like to implement this.
The text was updated successfully, but these errors were encountered: