Skip to content
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

Naive switch from Flask to AIOHTTP fails #578

Closed
hjacobs opened this issue Apr 9, 2018 · 9 comments
Closed

Naive switch from Flask to AIOHTTP fails #578

hjacobs opened this issue Apr 9, 2018 · 9 comments
Labels

Comments

@hjacobs
Copy link
Contributor

hjacobs commented Apr 9, 2018

I'm trying my example project (https://github.com/hjacobs/connexion-example) to switch to AIOHTTP, but it fails.

  1. I have to add only_one_api=True to the constructor in app.py (this probably deserves a documentation entry)
  2. server starts, but requests fail:
INFO:connexion.aiohttp_app:Listening on 0.0.0.0:8080..
======== Running on http://0.0.0.0:8080 ========
(Press CTRL+C to quit)
INFO:root:Creating pet 1..
ERROR:aiohttp.server:Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/aiohttp/web_protocol.py", line 381, in start
    resp = await self._request_handler(request)
  File "/usr/local/lib/python3.6/dist-packages/aiohttp/web_app.py", line 322, in _handle
    resp = await handler(request)
  File "/usr/local/lib/python3.6/dist-packages/aiohttp/web_middlewares.py", line 88, in impl
    return await handler(request)
  File "/usr/local/lib/python3.6/dist-packages/connexion/apis/aiohttp_api.py", line 32, in oauth_problem_middleware
    response = yield from handler(request)
  File "/usr/local/lib/python3.6/dist-packages/connexion/decorators/coroutine_wrappers.py", line 23, in wrapper
    connexion_response = yield from connexion_response
  File "/usr/local/lib/python3.6/dist-packages/connexion/apis/aiohttp_api.py", line 208, in get_response
    response.status, extra={'data': response.body, 'url': url})
AttributeError: 'tuple' object has no attribute 'status'
ERROR:aiohttp.server:Error handling request
@hjacobs hjacobs added the bug label Apr 9, 2018
@hjacobs
Copy link
Contributor Author

hjacobs commented Apr 9, 2018

/cc @dutradda

@dutradda
Copy link
Contributor

dutradda commented Apr 9, 2018

@hjacobs
we know about it and we had discussed about that:
#530 (comment)

The solution was add these parts on docs:
"Also check aiohttp handler examples"

So it is not a bug because the way how aiohttp handler works is different from the flask handler.
We can improve the documentation to expose this differences.

I can do an improve to support the same handler returns types as flask. But again, it is not a bug.

About the only_ony_api argument we can put in the docs improves too.

@prawn-cake
Copy link
Contributor

@hjacobs
I checked the changes with the test project and also tried naive approach which didn't work. It worked without only_one_api though, didn't have problems with that.

@dutradda we should probably make a separate section with the few short examples, I feel that ppl will expect it to be transparent, so we need to disillusion it with clear examples / hello world aiohttp connexion app.

@dutradda
Copy link
Contributor

@prawn-cake
I was thinking in improve the aiohttpApi.get_response to support others returns from the handlers.
These others returns will be the same as flask support, except a flaskResponse, of course.

Other approach is to implement these types of return types directly on the abstract class, so connexion will support natively.

@hjacobs
Copy link
Contributor Author

hjacobs commented Apr 10, 2018

@prawn-cake @dutradda please contribute an example app, e.g. into the examples directory. Thanks and great work! 😄

@prawn-cake
Copy link
Contributor

@dutradda not sure if I got what you mean. If the problem now that it's not intuitive to add those handlers right then I'd address it to the documentation which should be done anyway.
But if you see the room for other improvements, it's great, let's do that as well

ainquel added a commit to ainquel/connexion that referenced this issue Dec 20, 2018
Documented in AbstractApi._response_from_handler.
This method is implemented by subclasses.
Body and tuples are validated by connexion.operations.validation.validate_operation_output
fixes spec-first#578
@cognifloyd
Copy link
Contributor

cognifloyd commented Dec 4, 2019

unifying response handling (ie accepting tuples) between Flask and AioHttp is implemented in #849.

@cognifloyd
Copy link
Contributor

Even after #849 is merged, there will still be differences in serialization that will have to wait for v3.0 (WIP fix in #1095) because harmonizing serialization will require backwards incompatible changes for both flask and aiohttp.

@RobbeSneyders
Copy link
Member

aiohttp has been dropped and switch between Flask and AsyncApp is now straightforward.
#1395

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants