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

fix: solara run should only print url when the server is running #776

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maartenbreddels
Copy link
Contributor

If it takes a bit for a server to start, you will click the url and it will not work.

If it takes a bit for a server to start, you will click the url
and it will not work.
Copy link

render bot commented Sep 5, 2024

Copy link
Collaborator

@iisakkirotko iisakkirotko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple issues present (at least on my machine?) that seem a bit mysterious to me:

  • if I run solara run solara.website.pages -a only Solara server is starting... gets printed, even though the server itself runs fine, and the can be connected to
  • if I run solara run solara.website.pages, I get an internal server error, because of:
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 407, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 69, in __call__
    return await self.app(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/applications.py", line 123, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/middleware/errors.py", line 186, in __call__
    raise exc
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/middleware/errors.py", line 164, in __call__
    await self.app(scope, receive, _send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/middleware/gzip.py", line 24, in __call__
    await responder(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/middleware/gzip.py", line 44, in __call__
    await self.app(scope, receive, self.send_with_gzip)
  File "/Users/iisakki/Code/widgetti/solara/packages/solara-enterprise/solara_enterprise/auth/middleware.py", line 127, in __call__
    await self.app(scope, receive, send_wrapper)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/middleware/authentication.py", line 49, in __call__
    await self.app(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 65, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/routing.py", line 756, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/routing.py", line 776, in app
    await route.handle(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/routing.py", line 297, in handle
    await self.app(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/routing.py", line 77, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/_exception_handler.py", line 64, in wrapped_app
    raise exc
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/iisakki/micromamba/envs/solara-dev/lib/python3.10/site-packages/starlette/routing.py", line 72, in app
    response = await func(request)
  File "/Users/iisakki/Code/widgetti/solara/solara/server/starlette.py", line 432, in root
    content = server.read_root(request_path, root_path)
  File "/Users/iisakki/Code/widgetti/solara/solara/server/server.py", line 283, in read_root
    default_app = app.apps["__default__"]
KeyError: '__default__'

These issues are only present on this branch and don't happen on master. Although I'm also not sure if they are directly caused by the changes here or something else...

Comment on lines +368 to +369
# remove the Solara server is starting ... line and print the url (in green)
print(f"\r\x1b[1;32mSolara server is running at {url}\x1b[0m")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be nicer to leave the Solara server is starting... in, and print Solara server started at {url} in green after?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, maybe the \r can be removed.

def open_browser():
while not failed and (server is None or not server.started):
def check_server_started():
while not failed and (server is None or not server.started) or not solara.server.server.is_ready(url):
Copy link
Collaborator

@iisakkirotko iisakkirotko Sep 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think I've solved my second issue

if I run solara run solara.website.pages, I get an internal server error, because of:

This is due to the change in import order. Because we now import solara.server.server, which in turn imports from . import app, app.py runs before this line. Then app.apps is empty, and the server can't start.

For a solution, we could import solara.server.server here instead of at the top, although I'm not sure if this could introduce a race condition of some sort.

Suggested change
while not failed and (server is None or not server.started) or not solara.server.server.is_ready(url):
import solara.server.server
while not failed and (server is None or not server.started) or not solara.server.server.is_ready(url):

Another option would be to move this check down to below where we set environmental variables.

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.

2 participants