-
Notifications
You must be signed in to change notification settings - Fork 0
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
DM-43939: Create a bootcamp demo app showing basic path operations #2
Conversation
For this "external" router, we'll use it to demonstrate basic path operations, and keeping the models co-located with the path operation functions makes it a little easier to demonstrate. By getting rid of the models module, we also make space for a better architecture for separating internal and API models.
I think this was a bug in our template that'll be fixed in lsst/templates#253
Cover path parameters, query parameters, response models, and post requests with request models. Cover basic logging with Safir's logger_dependency.
97d75c0
to
cdcedd8
Compare
We'll need to modify the template to deal with hyphenated app names.
cdcedd8
to
9f284da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Really easy to read and a very natural flow. Great work!
@@ -27,7 +27,7 @@ class Config(BaseSettings): | |||
) | |||
|
|||
model_config = SettingsConfigDict( | |||
env_prefix="FASTAPI-BOOTCAMP_", case_sensitive=False | |||
env_prefix="FASTAPI_BOOTCAMP_", case_sensitive=False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in lsst/templates#254
# ============================================================================= | ||
# Lesson 1: A simple GET endpoint. | ||
|
||
|
||
@external_router.get("/hello", summary="Get a friendly greeting.") | ||
async def get_greeting() -> str: | ||
return "Hello, SQuaRE Services Bootcamp!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I agree with keeping this simple, but it could be a bit surprising that this handler still returns JSON, so the response will have quote marks around it. The simplest response from an HTTP perspective would require a bit more complexity in FastAPI to specify response_class=PlainTextResponse
. Maybe that's worth mentioning in a comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think about that, but mentioning this will definitely clarify things.
Rather than mixing JSON and query parameters in the same example, let's do the JSON response first, and then add in the query parameter next.
Covers path parameters, query parameters, response models, and post requests with request models. Cover basic logging with Safir's logger_dependency.
In the next PR, #3, we add to this with a realistic app to show the sqr-072.lsst.io-type architecture.