-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Remove loading of Copilot Arena tab when env variable is not set #3645
Remove loading of Copilot Arena tab when env variable is not set #3645
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
fastchat/serve/monitor/monitor.py
Outdated
try: | ||
with gr.Tab("Copilot Arena Leaderboard", id=5): | ||
from fastchat.serve.monitor.copilot_arena import ( | ||
build_copilot_arena_tab, | ||
) | ||
from fastchat.serve.monitor.copilot_arena import ( | ||
build_copilot_arena_tab, | ||
copilot_arena_leaderboard_url, | ||
) | ||
|
||
if not copilot_arena_leaderboard_url: | ||
raise ValueError( | ||
"COPILOT_ARENA_LEADERBOARD_URL environment variable is not set. " | ||
"Please configure it to a valid URL." | ||
) | ||
with gr.Tab("Copilot Arena Leaderboard", id=5): | ||
build_copilot_arena_tab() | ||
except Exception as e: | ||
print(f"Unable to build Copilot Arena's Leaderboard. Error: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you think we should do try catch or just if else like below
if copilot_arena_leaderboard_url:
with gr.Tab("Copilot Arena Leaderboard", id=5):
build_copilot_arena_tab()
else:
print(f"Unable to build Copilot Arena's Leaderboard")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ Agreed with this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I've just updated this
Why are these changes needed?
Adding a small change to #3618, which doesn't load the Copilot Arena leaderboard tab when the environment variable is not set.
Related issue number (if applicable)
Checks
format.sh
to lint the changes in this PR.