-
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
Wip 369 doc to models #227
Conversation
3ed9a30
to
21083b8
Compare
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
2c99ddd
to
b774abb
Compare
b774abb
to
fa423a9
Compare
|
||
|
||
# Server error responses for DP queries | ||
SERVER_QUERY_ERROR_RESPONSES: dict[int | str, dict[str, Any]] = { |
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.
What is the goal of this and ...ExceptionModel exceptions versus what was there before ?
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 allows to list the possible responses that an endpoint can return:
@router.post(
"/dummy_diffprivlib_query",
dependencies=[Depends(server_live)],
response_model=QueryResponse,
responses=SERVER_QUERY_ERROR_RESPONSES,
tags=["USER_DUMMY"],
)
The format of the dict in responses requires pydantic models. Using these also has the benefit that we can show the output model in the redoc documentation, not only for the standard response, but also for exceptions.
class ExternalLibraryExceptionModel(LomasServerExceptionModel): | ||
"""For exceptions from libraries external to the lomas packages.""" | ||
|
||
type: Literal[ExceptionType.EXTERNAL_LIBRARY] = ExceptionType.EXTERNAL_LIBRARY | ||
"""Exception type.""" | ||
library: DPLibraries | ||
"""The external library that caused the exception.""" | ||
message: str | ||
"""Exception error message. | ||
|
||
For exceptions from libraries external to the lomas packages. | ||
""" | ||
|
||
|
||
class UnauthorizedAccessExceptionModel(LomasServerExceptionModel): |
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.
aren't ExternalLibraryException and UnauthorizedAccessException always related to a query ? (just trying to understand)
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 am not sure, maybe we should clarify this somewhere. How would you use them?
- Unauthorized: every time the user does not have access?
- ExternalLibrary: only when we catch an exception from an external library
- ...?
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.
It is here: https://dscc-admin-ch.github.io/lomas-docs/client_errors.html
Except for InternalServerException, I think all exceptions are always directly related to queries.
It looks good! |
…current branch. Move endpoint documentation to pydantic models. Add responses to path operators and add models to exceptions. Modify client accordingly Moved example queries out of default values to model configs. Update docummentation of routes/utils methods.
fa423a9
to
4c506c2
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.
Very clean, thanks!
Moved documentation to pydantic models.
Redoc only shows first part of docstring (until \f character), while sphinx doc the entire docstring (see pictures).
Also added core documentation to sphinx and option to build locally.