refactor(ui): mm model install error handling #7512
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
psychedelicious
requested review from
blessedcoolant,
maryhipp,
hipsterusername,
brandonrising and
lstein
as code owners
github-actions
bot
added
python
maryhipp
approved these changes
Jan 3, 2025
hipsterusername
approved these changes
Jan 3, 2025
No logic change.
This is required to fix an issue with the MM UI's error handling. Previously, we only included the model source as a string. That could be an arbitrary URL, file path or HF repo id, but the frontend has no parsing logic to differentiate between these different model sources. Without access to the type of model source, it is difficult to determine how the user should proceed. For example, if it's HF URL with an HTTP unauthorized error, we should direct the user to log in to HF. But if it's a civitai URL with the same error, we should not direct the user to HF. There are a variety of related edge cases. With this change, the full `ModelSource` object is included in each model install event, including error events. I had to fix some circular import issues, hence the import changes to files other than `events_common.py`.
…arately Previously, we didn't differentiate between model install errors for different types of model install sources, resulting in a buggy UX: - If a HF model install failed, but it was a HF URL install and not a repo id install, the link to the HF model page was incorrect. - If a non-HF URL install (e.g. civitai) failed, we treated it as a HF URL install. In this case, if the user's HF token was invalid or unset, we directed the user to set it. If the HF token was valid, we displayed an empty red toast. If it's not a HF URL install, then of course neither of these are correct. Also, the logic for handling the toasts was a bit complicated. This change does a few things: - Consolidate the model install error toasts into one place - the socket.io event handler for the model install error event. There is no more global state for the toasts and there are no hooks managing them. - Handling the different cases for errors, including all combinations of HF/non-HF and unauthorized/forbidden/unknown.
For example, "flux" now matches any starter model with a model base of "FLUX".
…y URL and not repo id
hipsterusername
force-pushed
the
psyche/feat/mm-auth-handling
branch
from
a5a796f to
9525c38
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses a number of issues with the handling of model install errors:
Other changes in this PR:
invokeai.yamlwhen downloading from HF URLs.See the commit messages for technical details.
Related Issues / Discussions
QA Instructions
This is a bit tricky to test. We need to test both HF repo ID install sources and "direct" URL install sources, and the 3 types of errors (unauthorized - no API token, forbidden - has API token but didn't agree to TOS, or something else like 404).
The FLUX Canny LoRA is a good test case because it requires the user to agree to BFL's TOS to download:
black-forest-labs/FLUX.1-Canny-dev-lora::flux1-canny-dev-lora.safetensorshttps://huggingface.co/black-forest-labs/FLUX.1-Canny-dev-lora/resolve/main/flux1-canny-dev-lora.safetensorsTo test the error types, I needed to create a tester HF account that has an API key but hadn't yet agreed to the BFL agreement for the FLUX Canny LoRA.
When testing a Repo ID, we use the token stored in
~/.cache/huggingface/token. I deleted this file for each test, and entered my token in the MM UI (requires refreshing the browser).When testing a URL - even a HF URL - we use the token in the
remote_api_tokenssetting ininvokeai.yaml. You need to restart the server if you change this.I did not test civitai URLs, but it should be exactly the same as HF URLs, because internally it's just using the python
requestsmodule or whatever generic network module. The error toasts are also the same for all URLs. HF URLs do not get any special handling.Expected results
invokeai.yaml: error toast directs user to our docs to set up the token ininvokeai.yamlinvokeai.yaml, but user still got forbidden: error toast directs user to request access from whoever is hosting the modelI got the expected results in my testing.
Merge Plan
n/a
Checklist