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

Added portable Uvicorn class #401

Closed
wants to merge 7 commits into from
Closed

Added portable Uvicorn class #401

wants to merge 7 commits into from

Conversation

databasedav
Copy link

My use is specifically for loop extraction to other apps, but other functionality can be added.

Includes an example in the docs using FastAPI and Faust.

@florimondmanca
Copy link
Member

Hi,

I quickly took a look at the code, and it looks like you're forcing the use of asyncio there. But uvicorn does not always run on asyncio — it may run on asyncio or uvloop (see Settings - Implementation).

I'm wondering, though — doesn't Faust support using whichever event loop is currently configured? I believe it should simply call asyncio.get_event_loop() when it needs to access it, instead of requiring to pass the loop upon instanciation. Perhaps this would be worth discussing on the Faust repo?

@databasedav
Copy link
Author

databasedav commented Aug 8, 2019

I'm not forcing the use of asyncio since Config.setup_event_loop sets the event loop policy to the one of your choosing. I haven't changed any behavior, just wrapped run a in class for a tiny bit of convenience really.

And yes, I believe Faust does do this and I can't figure out why (doesn't look like Faust creates a new event loop anywhere; it may be because Uvicorn does?) but in my testing simply doing

import asyncio
import uvloop
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
from faust import App
import uvicorn

faust_app = App(..., loop=asyncio.get_event_loop())

...

if __name__ == '__main__':
    uvicorn.run(...)

leads to RuntimeError: Task ... running at ... got Future <Future pending> attached to a different loop errors, while extracting the event loop from the Uvicorn instance like I do in the docs does not.

@florimondmanca
Copy link
Member

Oh, yes, my bad, I didn’t realize uvloop is just a different loop that still allows using asyncio everywhere else.

@tomchristie
Copy link
Member

Closing this in favor of #455 which I think allows you the same functionality without us having to introduce much more API. Does that work okay for you?

@databasedav
Copy link
Author

This actually doesn't work because there's no way to extract the event loop prior to server.run() since the loop is created then. I'm not sure what you meant by "setup the loop yourself" here #442 (comment) since the loop arg for Config takes a string for the type of loop but not the loop object itself. But I agree that there's no need to introduce more API then needed so I suggest one of the following.

  • Allow Config's loop arg to take an event loop object so we actually can setup the loop ourselves and pass it to whatever we want (which is the issue pointed out in Extracting event loop reference to synchronise with Faust #442)
  • scrap the Uvicorn class but keep my additions to Server from this PR, i.e. this stuff:
class Server:
    def __init__(self, config):
        self.config = config
        self.server_state = ServerState()

        self.started = False
        self.should_exit = False
        self.force_exit = False
        self.last_notified = 0
        # begin stuff
        self.loop = None
        self.event_loop_setup = False
    
    def setup_event_loop(self):
        self.config.setup_event_loop()
        self.event_loop_setup = True

    def get_event_loop(self):
        if not self.event_loop_setup:
            self.setup_event_loop()
        if not self.loop:
            self.loop = asyncio.get_event_loop()
        return self.loop

    def run(self, sockets=None, shutdown_servers=True):
        loop = self.get_event_loop()
        loop.run_until_complete(self.serve(sockets=sockets))
        # end stuff

I think the former may make more sense but is inconsistent with the current API. Please let me know what you think. Thanks!

@lightoyou
Copy link

ReOpen it please

@euri10
Copy link
Member

euri10 commented Oct 12, 2020

ReOpen it please

why ?

@lightoyou
Copy link

Is it' working ? I try to use uvicorn (fastapi) and faust maybe. I need an example. Cause it's not working for me.

@euri10
Copy link
Member

euri10 commented Oct 12, 2020

something like this should, feel free to ask precise questions or join the chat to get help, it's not working is not helpful

config = Config(app=fastapi_app, loop=loop_instance)
server = Server(config=config)
server.run()

@lightoyou
Copy link

Yeah i join the chat to get more help :)

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

Successfully merging this pull request may close these issues.

5 participants