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

@render.download not namespacing properly, used to work in deprecated @session.download #1724

Closed
nsiicm0 opened this issue Oct 9, 2024 · 0 comments · Fixed by #1732
Closed
Assignees
Labels
bug Something isn't working

Comments

@nsiicm0
Copy link
Contributor

nsiicm0 commented Oct 9, 2024

Hi all,
I faced an issue in one of our applications, where when using @render.download the generated URLs are not properly namespaced (actually the download URL appears to be living in "root" added at all).
This is usually fine, however, in our application, I am trying to modularize the download capabilities and embed it on multiple subpages (up to 10 or more times), hence, it will become an issue, since every embedding will produce a different download.

I am using shiny version 1.1.0.

Here is an example app illustrating my issue:

app.py

import shiny
from shiny import App, Inputs, Outputs, Session, reactive, render, ui, module
from starlette.applications import Starlette
from starlette.routing import Route, Mount

from download import download_ui, renderer_download_server, session_download_server

version = shiny.__version__

app_ui = ui.page_navbar(  
    ui.nav_panel("Home", ui.TagList([
        download_ui('download_renderer')
        , ui.tags.br()
        , download_ui('download_session')
    ])), 
    title=f"Download Bug App - Shiny Version {version}",  
    id="page",  
)  

def server(input, output, session):
    renderer_download_server('download_renderer')
    session_download_server('download_session')

shiny_app = App(app_ui, server, debug=True)
shiny_app.sanitize_errors = False

routes = [
    Mount('/', app=shiny_app)
]

app = Starlette(routes=routes, debug=True)

download.py

from shiny import module, ui, render, event, reactive

@module.ui
def download_ui():
    return ui.download_button('dwnld', "Download")

@module.server
def renderer_download_server(input, output, session):
    @render.download(filename=lambda: f"test.txt")
    def dwnld():
        ui.notification_show(f'Download using renderer started | {session.ns=}')
        
@module.server
def session_download_server(input, output, session):
    @session.download(filename=lambda: f"test.txt")
    def dwnld():
        ui.notification_show(f'Download using session started | {session.ns=}')

This app will produce two buttons. When hovering over the first button (that uses the @render.download functionality) then the link will look like session/<id>/download/dwnld?w=, however, the second button (@session.download) will produce a namespaced subpath that looks like session/<id>/download/download_session-dwnld?w= (see screenshot below of the DOM).
Image

This namespaced variation is important when implementing multiple download buttons that are being generated with a module. Otherwise a click on any of the generated module download buttons will trigger the "last" registered download handler function.
To illustrate this, see this modified example:

## app.py
app_ui = ui.page_navbar(  
    ui.nav_panel("Home", ui.TagList([
        download_ui('download_renderer')
        , ui.tags.br()
        , download_ui('download_session')
        , ui.tags.br()
        , download_ui('download_renderer2')
    ])), 
    title=f"Download Bug App - Shiny Version {version}",  
    id="page",  
)  
def server(input, output, session):
    renderer_download_server('download_renderer', value='Foo')
    session_download_server('download_session', value='Bar')
    renderer_download_server('download_renderer2', value='Baz')

## download.py
@module.server
def renderer_download_server(input, output, session, value):
    @render.download(filename=lambda: f"test.txt")
    def dwnld():
        ui.notification_show(f'Download using renderer started {value=} | {session.ns=}')
        
@module.server
def session_download_server(input, output, session, value):
    @session.download(filename=lambda: f"test.txt")
    def dwnld():
        ui.notification_show(f'Download using session started {value=} | {session.ns=}')

This should output (notification) "Foo" on the first button, "Bar" on the second button, and "Baz" on the third button. However, when clicking on the first button I get:
Image
Additionaly, as you can see, the session namespace is a entirely different one (the namespace of the third button). This flaw can be quite dangerous, as it messes up everything.

On top of this, the @render.download does not allow me to specify an "id" as it was the case with @session.download, which can be used to override the automatic linking of a download handler function to a download button.

As a workaround I managed to use @session.download in combination with the id= parameter with a pinch of ResolvedId magic, however, this inconsistency and also the deprecation warning in @session.download gives me quite some headaches for the future.
See the following simplified snippet of what I mean by this previous sentence:

from shiny._namespaces import ResolvedId
parent = 'test' # provided by calling module

@module.ui
def patched_download_ui():
    return ui.download_button(ResolvedId(f'{parent}-dwnld'), "Patched Download")

@module.server
def patched_download_server(input, output, session):
    @session.download(id=ResolvedId(f'{parent}-dwnld'), filename=lambda: f"test.txt")
    def dwnld():
        ...

With this ResolvedId method, I am fully in control which methods I want to tie to which button.

How to resolve this?

I think it would be best to...

  • ... make sure that @render.download properly namespaces the button/subpaths similar to @session.download
  • ... introduce the id= parameter in @render.download

What do you guys think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants