Skip to content

feat(server): Add support for async server function, module.server, and express module #1842

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.

Sign up for GitHub

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat(server): Add support for async server function, module.server, and express module #1842

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3662f6c
add support for async server function
mconflitti-pbc Feb 5, 2025
04bf6b8
lint fixes
mconflitti-pbc Feb 5, 2025
aaa9c6e
updated annotations; make server async regardless of function passed in
mconflitti-pbc Feb 5, 2025
7162f5d
lint fixes
mconflitti-pbc Feb 5, 2025
f1aa19c
py3.9 needs union for cast
mconflitti-pbc Feb 5, 2025
88bdf4b
lint fixes
mconflitti-pbc Feb 5, 2025
03721bb
Merge branch 'main' into mconflitti/allow-async-server-func
schloerke Mar 28, 2025
58c0db5
Allow for module server to be async
schloerke Mar 28, 2025
eacb07b
Allow for express module server to be async
schloerke Mar 28, 2025
d6b2420
Uncomment `not_is_async_callable()`
schloerke Mar 28, 2025
cbb3242
lints
schloerke Mar 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions shiny/_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from contextlib import AsyncExitStack, asynccontextmanager
from inspect import signature
from pathlib import Path
from typing import Any, Callable, Mapping, Optional, TypeVar, cast
from typing import Any, Awaitable, Callable, Mapping, Optional, TypeVar, cast

import starlette.applications
import starlette.exceptions
Expand Down Expand Up @@ -57,8 +57,8 @@ class App:
returns a UI definition, if you need the UI definition to be created dynamically
for each pageview.
server
A function which is called once for each session, ensuring that each session is
independent.
A sync or async function which is called once for each session, ensuring that
each session is independent.
static_assets
Static files to be served by the app. If this is a string or Path object, it
must be a directory, and it will be mounted at `/`. If this is a dictionary,
Expand Down Expand Up @@ -104,13 +104,15 @@ def server(input: Inputs, output: Outputs, session: Session):
"""

ui: RenderedHTML | Callable[[Request], Tag | TagList]
server: Callable[[Inputs, Outputs, Session], None]
server: Callable[[Inputs, Outputs, Session], Awaitable[None] | None]

def __init__(
self,
ui: Tag | TagList | Callable[[Request], Tag | TagList] | Path,
server: (
Callable[[Inputs], None] | Callable[[Inputs, Outputs, Session], None] | None
Callable[[Inputs], Awaitable[None] | None]
| Callable[[Inputs, Outputs, Session], Awaitable[None] | None]
Comment on lines +136 to +137
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cosmetic split.

Suggested change
Callable[[Inputs], Awaitable[None] | None]
| Callable[[Inputs, Outputs, Session], Awaitable[None] | None]
Callable[[Inputs], None]
| Callable[[Inputs, Outputs, Session], None]
| Callable[[Inputs], Awaitable[None]]
| Callable[[Inputs, Outputs, Session], Awaitable[None]]

I remember something from past return types that it's a little better to split Awaitable from the return type as it can't actually return both, but it is really two independent types.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, they are different types and need separate entries for correctness.

| None
Comment on lines +136 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because this can be defined with an async function, can we use wrap_async when storing the fn value below?

This will always upgrade the function to async. Then in _session.py, we will always await the server execution as the run method is already an async method.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The current cast calls are lying as to what values are already there. Please update them accordingly.

By wrapping all server functions to be async, we won't need to check if we need to await or not (as we'll always need to await).

Copy link
Author

Choose a reason for hiding this comment

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

Sweet. Makes sense to me.

Copy link
Author

Choose a reason for hiding this comment

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

Though, now looking into support server function to be async, would need to go down the rabbit hole of ensure that we tackle other side effects like the module.server() decorator which is not async. Could leave it only supporting sync i suppose.. curious on thoughts from @wch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Happy to have all server-like functions upgraded to async

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @schloerke - it makes sense to make everything async-capable. Hopefully that's not too much more work.

),
*,
static_assets: Optional[str | Path | Mapping[str, str | Path]] = None,
Expand Down
4 changes: 3 additions & 1 deletion shiny/session/_session.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,9 @@ def verify_state(expected_state: ConnectionState) -> None:
self._manage_inputs(message_obj["data"])

with session_context(self):
self.app.server(self.input, self.output, self)
result = self.app.server(self.input, self.output, self)
if isinstance(result, Awaitable):
await result

elif message_obj["method"] == "update":
verify_state(ConnectionState.Running)
Expand Down
Loading