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

Removing global to make it gradio usable across multiple sessions #186

Open
xhluca opened this issue Dec 18, 2024 · 7 comments
Open

Removing global to make it gradio usable across multiple sessions #186

xhluca opened this issue Dec 18, 2024 · 7 comments

Comments

@xhluca
Copy link
Contributor

xhluca commented Dec 18, 2024

https://github.com/ServiceNow/AgentLab/blob/3c123811238879ff117c0d5b95d1da2db2101524/src/agentlab/analyze/agent_xray.py#L591C5-L591C16

It is not currently possible to use agent xray across multiple tabs, as it causes desync issue. I suspect it is caused by the use of global above

@recursix
Copy link
Collaborator

recursix commented Jan 6, 2025

using module variables is generally not a good practice. With gradio, it's the only way I found to efficiently pass an object to the functions without it being a gradio data. Removing global would break the whole UI.

To achieve multi-tabs, you need to launch multiple gradio instance. I think we would have to let it chose a port though (here)

@recursix
Copy link
Collaborator

recursix commented Jan 6, 2025

perhaps something like this:

import os

do_share = os.getenv("AGENTXRAY_SHARE_GRADIO", "false").lower() == "true"
port_env = os.getenv("AGENTXRAY_APP_PORT", None)

if port_env:
    # Use the specified port
    demo.launch(server_port=int(port_env), share=do_share)
else:
    # Let Gradio pick the port
    demo.launch(share=do_share)

@xhluca
Copy link
Contributor Author

xhluca commented Jan 6, 2025

Thanks, I am currently launching multiple instances, which mitigates the problem.

One concern is that the xray ui might be used by multiple viewers (e.g. huggingface spaces), it might not be possible to spin 10 instances if there are 10 viewers, for example.

without it being a gradio data

Thanks, is there any concern why we should avoid using gradio data? I have found gradio state to work fairly well! I would assume it is stored server side (if it's client side, i understand hte concern)

@recursix
Copy link
Collaborator

recursix commented Jan 6, 2025

I made tests in the past with gradio state and it was clunky. Which is why I decided to go with a global. I don't remember what was the issue with gradio state though.

@xhluca
Copy link
Contributor Author

xhluca commented Jan 7, 2025

perhaps they fixed it in the newest gradio. however changing from global to state would take a lot of refactoring effort. understandable that we don't want to do it right now (perhaps in the future when sharing an xray with a lot of users on spaces)

@ThibaultLSDC
Copy link
Collaborator

Now that we have such spaces for showcase (gradio demo), maybe we could restart a discussion on the topic ?

@xhluca
Copy link
Contributor Author

xhluca commented Feb 7, 2025

Agree. I'm sharing agent X-ray results to collaborators but have to tell them to refresh if they run into issues with desync. It would be a great ux improvement to fix global

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

No branches or pull requests

3 participants