AP-856: scaffold a basic flask+celery app - #1
Conversation
caf0ada to
ace4e82
Compare
4986e98 to
312f342
Compare
* creates a simple app with a 2 routes: / and /health * creates a celery app that can be extended further * sets up Docker and Compose configuration to spin up the necessary services: db, app, worker, and redis * adds github actions workflows
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
r+wc: There's a lot of missing docstrings, but perhaps that's acceptable here. I didn't notate each one. Most of our other Python projects have both module- and function-level docstrings, so it was a bit surprising to have none here. I don't know if we need to manually pull in redis in PyProject. Tiny style nits. Otherwise, this looks good.
| <head><title>quiabo</title></head> | ||
| <body> | ||
| <img src="{{ url_for('static', filename='quiabo.png') }}" alt="quiabo logo"/> | ||
| <h1>Goodbye Doggy!</h1> |
|
|
||
| app = create_app() | ||
| celery_app = celery_init_app(app) | ||
|
|
| app = Flask(__name__) | ||
| app.config.from_prefixed_env(prefix="QUIABO") | ||
|
|
||
| app.register_blueprint(root.bp) |
There was a problem hiding this comment.
Do we want to call this Root, or possibly Application? Also, I don't know if we want to register this one last, so that other blueprints won't be shadowed by it in case of accidental shadowing. At least, I always felt that was less surprising behaviour - we can leave it in this order, as I doubt that will happen here.
There was a problem hiding this comment.
it's sort of a weird example. we don't have anything else hanging off the / Blueprint, so it's kind of strange.
|
|
||
| # other stuff | ||
| artifacts/* | ||
| uv.lock No newline at end of file |
| @@ -0,0 +1,47 @@ | |||
| FROM python:3.14-slim AS reqs | |||
There was a problem hiding this comment.
Do we want Python version to be an ARG?
There was a problem hiding this comment.
we haven't yet done that for other python apps but i'm not opposed.
| "flower", | ||
| "gunicorn", | ||
| "psycopg2", | ||
| "redis", |
There was a problem hiding this comment.
I don't see redis being used by the app itself. celery should pull it in if it's using it itself, right?
There was a problem hiding this comment.
celery has many brokers and result backends, so by default it's light and doesn't pull in any client dependencies. this could be specified as celery[redis,sqlalchemy] instead, but that's ultimately resolving next to kombu[redis] and kombu[sqlalchemy].
There was a problem hiding this comment.
Makes sense; we can just leave redis and sqlalchemy in the base deps. Maybe add a comment that it's for Celery backends.
|
i'll take a pass to add docstrings before merging and make the requested changes. |
2 participants
/and/health, and integrates celery into it