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

Add support for async server function #1842

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

Conversation

mconflitti-pbc
Copy link

Coming across a use case using chatlas where I need to be able to await an async function in my server function but shiny does not currently support this.

Simple addition to _session.py to handle both sync and async.

@schloerke
Copy link
Collaborator

Is your use case for setting up data? When are you needing the result of the information? (I guess, can you give a tiny motivation example that chatlas would use?)

Why not put it behind an async reactive calculation? Then the value can be used accordingly.

Ex:

From

async def server(input):
    data = await get_data()

to

def server(input):

    @reactive.calc
    async def data():
        return await get_data()

@mconflitti-pbc
Copy link
Author

Is your use case for setting up data? When are you needing the result of the information? (I guess, can you give a tiny motivation example that chatlas would use?)

Why not put it behind an async reactive calculation? Then the value can be used accordingly.

Ex:

From

async def server(input):
    data = await get_data()

to

def server(input):

    @reactive.calc
    async def data():
        return await get_data()

Inside of chatlas, the server function is defined for you and I want to be able to access the session headers to get the user session token when using chat.app()

in chat.app()

  async def server(input, output, session):  # noqa: A002
      if session_callback:
          await session_callback(session)

May be too niche of a problem, but was surprised to see shiny didnt support both. And afaict, no reason that it can't.

Comment on lines +113 to +115
Callable[[Inputs], Awaitable[None] | None]
| Callable[[Inputs, Outputs, Session], Awaitable[None] | None]
| None
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.

@schloerke
Copy link
Collaborator

was surprised to see shiny didnt support both. And afaict, no reason that it can't.

Fair!

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