-
-
Notifications
You must be signed in to change notification settings - Fork 767
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
Update examples for Connexion 3.0 #1615
Conversation
56e0c24
to
72dffa1
Compare
72dffa1
to
90b2908
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.
Self review.
@@ -20,9 +20,7 @@ def __init__( | |||
self, | |||
import_name, | |||
api_cls, | |||
port=None, |
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 see no reason to pass these to the app instead of to app.run
directly.
@@ -83,7 +83,11 @@ async def asgi_app(self, scope: Scope, receive: Receive, send: Send) -> None: | |||
) | |||
|
|||
api_base_path = connexion_context.get("api_base_path") | |||
if api_base_path and not api_base_path == self.base_path: | |||
if ( |
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.
Handle case when base path is "", which is the case both when endpoint is not part of an API, or when endpoint is part of the API registered on the root path.
@@ -124,89 +118,11 @@ def index(): | |||
logger.debug("Adding %s with decorator", rule, extra=kwargs) | |||
return self.app.route(rule, **kwargs) | |||
|
|||
def run( |
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.
Lifted to AbstractApp.
@@ -38,6 +38,7 @@ def parameter_to_arg( | |||
sanitize = pythonic if pythonic_params else sanitized | |||
arguments, has_kwargs = inspect_function_arguments(function) | |||
|
|||
# TODO: should always be used for AsyncApp |
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 needs some work. Will submit a separate PR for this.
@@ -55,7 +55,7 @@ def _apply_middlewares( | |||
for middleware in reversed(middlewares): | |||
app = middleware(app) # type: ignore | |||
apps.append(app) | |||
return app, reversed(apps) | |||
return app, list(reversed(apps)) |
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.
Make sure iterator doesn't run out when registering multiple apis.
@@ -42,16 +43,11 @@ def __init__(self, *args, default: ASGIApp, **kwargs): | |||
def normalize_string(string): | |||
return re.sub(r"[^a-zA-Z0-9]", "_", string.strip("/")) | |||
|
|||
def _base_path_for_prefix(self, request): | |||
def _base_path_for_prefix(self, request: StarletteRequest) -> str: |
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 for proper usage of root_path behind reverse proxy.
@@ -2,6 +2,8 @@ openapi: 3.0.0 | |||
info: | |||
title: API Key Example | |||
version: '1.0' | |||
servers: | |||
- url: /openapi |
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.
We need a base path now that we're registering two apis on the same app.
info: | ||
title: Basic Auth Example | ||
version: "1.0" | ||
|
||
basePath: /swagger |
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.
We need a base path now that we're registering two apis on the same app.
|
||
|
||
def echo(data): | ||
# TODO: should work as sync endpoint when parameter decorator is fixed |
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.
Currently the parameter decorator uses a sync decorator when the view function is sync, even when the framework is async. This doesn't work as the async body cannot be read by a sync decorator.
@@ -100,7 +104,8 @@ def readme(): | |||
'tests': tests_require, | |||
'flask': flask_require, | |||
'swagger-ui': swagger_ui_require, | |||
'docs': docs_require | |||
'docs': docs_require, | |||
'uvicorn': uvicorn_requires, |
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.
Install with connexion[uvicorn]
to enable app.run
. The standard
extra includes the dependencies needed to watch files for reloading the app.
Pull Request Test Coverage Report for Build 3796655632
💛 - Coveralls |
https://github.com/spec-first/connexion/blob/main/examples/jwt/app.py the following example is targeted at FlaskApp would the same work out of the box by just replacing with AsyncApp ? |
Yes it should @Subin-Qreative! |
This PR updates the examples for Connexion 3.0 and merges them for OpenAPI and Swagger.
2 examples required some changes to make them work:
root_path
correctly. This is included in the PR.