-
-
Couldn't load subscription status.
- Fork 780
Add example of using the Starlette test client with tox #2026
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
base: main
Are you sure you want to change the base?
Conversation
bd956aa to
a6706ef
Compare
|
Thanks for approving a run of the test pipeline on this PR. I botched the import order in my initial attempts, I'm learning to run |
a6706ef to
b0d2d6a
Compare
b0d2d6a to
a2e4702
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.
Thanks! Already looking very nice, left some small comments/questions
| :return: ConnexionResponse with RFC7087 problem details | ||
| """ | ||
| # log the request URL, exception and stack trace | ||
| logger.exception("Request to %s caused exception", request.url) |
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.
Does this work properly, don't we need to supply the exception as an argument via exc_info=ex?
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.
The python logger does magic when you call its exception method, and that magic breaks when the context changes, like when running the function from a new thread pool. I will test what you suggest - pass the exception - and see if the full stack trace is shown.
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.
Thank you for @Ruwann for pointing me to the exc_info parameter. Yes, the stack trace appears as expected when that parameter is passed. That seems like a better solution than using async to define the exception handler function.
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 do you think of renaming this file instead of using __init__.py? Or add it to app.py? To keep it more consistent with other examples
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.
Would appreciate guidance on the right way, or the connexion way, or what have you :) to structure an app for use from tox. Creating the app object in __init__.py makes that easy but I'm sure it's not the only way.
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 restructured this example to use a single file, and eliminate the package hello. Please review again. Thanks in advance.
examples/testclient/hello/app.py
Outdated
| from . import conn_app | ||
|
|
||
| # reuse the configured logger | ||
| logger = logging.getLogger("uvicorn.error") |
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's the reasong for using the uvicorn.error logger instead of a specific one for this file/application?
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 was basically a cheat, the level is set appropriately on this logger so the output from my code appears together with output from uvicorn, and no additional configuration is necessary.
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.
Please say if using the unicorn.error logger is a deal breaker and I will change the logging.
No worries, I have been using ruff on my other projects as it combines a lot of these into a single tool, that's more easy to use imo. Is it still unclear? |
cd037f2 to
3088c4d
Compare
2ac0347 to
3ade3ab
Compare
4f93a0a to
4f21e7b
Compare
4f21e7b to
220d57f
Compare
|
Greetings @Ruwann and @RobbeSneyders I think I addressed all your comments on this PR, please advise if you would like this example in the V3 codebase, thanks. |
|
This PR addresses issue #1988 with an example instead of a revised document. Please comment. |
220d57f to
74a45e5
Compare
This example Connexion app processes JSON requests and responses as specified with OpenAPI v2 (aka Swagger) or OpenAPI v3 file format. The app asks Connexion to validate JSON responses against the spec. The app defines an error handler that catches exceptions raised while processing a request. Define automated tests that are run by tox.
74a45e5 to
b7ff39e
Compare
|
Looks like two my PRs are fighting with each other :( This PR adds a "tests" directory, it's the only example with that. Unfortunately the very recently merged change to |
|
I submitted a PR to restore the |
The example Connexion app processes JSON requests and responses as specified with OpenAPI v2 (aka Swagger) or OpenAPI v3 file format.
JSON responses are validated against the spec.
An error handler catches exceptions raised while processing a request.
Tests are run by
toxwhich also reports code coverage.Fixes: #1988