-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix: config loading set to false before being fetched #7994
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
base: main
Are you sure you want to change the base?
Conversation
gui/src/components/AssistantAndOrgListbox/SelectedAssistantButton.tsx
Outdated
Show resolved
Hide resolved
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.
@uinstinct I'm wondering if this will lead to infinite loading for local folks or folks with e.g. an error in their yaml schema. Why is the method for detecting config not yet loaded to check configLoadInterrupted
?
Rechecked it. It does not lead to infinite loading in incorrect yaml schema demo.mp4The reason why configLoadInterrupted is true is because profileId is null. We do the initial fetch and load of the config in the ConfigHandler constructor call. In case of local only config, this is immediately available (since core is initialized before gui and that ConfigLoader constructor is already called). In case of remote config, it takes a while to initialize due to the http calls (and config/getSerializedProfileInfo is already called in gui). |
@uinstinct in your video, it doesn't lead to infinite loading because a profile Id was selected. I understand that profileId === null and configLoadingInterrupted = true can be used to detect loading conditions with the current profileId setup, but the condition feels fragile since configLoadingInterrupted = true in other cases as well, e.g. Can we come up with a more robust way to detect this? |
Description
Do not set config loading to false before config from hub is fetched.
In case of local config, config is already loaded in core before gui initializes (due to the constructor call).
In case of profile config, config takes time to load (due to data fetching from remote server) and profileId is not yet set.
resolves CON-3946
AI Code Review
@continue-general-review
or@continue-detailed-review
Checklist
Screen recording or screenshot
before.mp4
after.mp4
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Fixes premature clearing of the config loading state when fetching config from the hub. The UI now stays in “Loading” until remote profile config is ready, preventing the empty-state flash and addressing CON-3946.