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

feat(mm): hide missing models from API response #6122

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

psychedelicious
Copy link
Collaborator

Summary

Check if the model file exists. If not, omit it from the list of models.

Related Issues / Discussions

Closes #6117

QA Instructions

Delete/rename/move an installed model. Refresh the UI and it should not appear in the list of models.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable) n/a
  • Documentation added / updated (if applicable) n/a

Check if the model file exists. If not, omit it from the list of models.

Closes #6117
@hipsterusername
Copy link
Member

Does this really solve the problem?

How does a user fix the missing model? Re-add?

@blessedcoolant
Copy link
Collaborator

blessedcoolant commented Apr 3, 2024

Not a solution imo. Coz if they no longer show up in the list, then the user cannot even remove the models from the database for them to be detected fresh. And as long the models exist in the root folder but not in the exact path, they will not be re-added coz the model name are the same. And they'll just get skipped over.

They need to show up and ideally be also marked as not found. So the user at least knows to delete the entries and let the auto importer do its job next boot. Even better, providing a route to update the path from the UI itself.

@psychedelicious
Copy link
Collaborator Author

psychedelicious commented Apr 3, 2024

I don't support this change (or any change related to the "missing models" edge case). We should never end up with models missing unless the user sticks their fingers in the pie. We can't both fully manage the model files and let users manage them.

There is no auto importer. We cannot remove missing models automatically anyways. For example if you disconnect your external drive, all your models are now missing and they get removed.

Tbh the current behaviour makes the most sense to me - fail loudly when a model is missing and log a warning in the terminal on startup.

In response to the issue vic made, I started looking into what would be involved with indicating/handling throughout the app when a models file are missing and it's a lot of work with fiddly UI. I ended up with this small change as the most time I want to spend on this edge case.

@psychedelicious psychedelicious marked this pull request as draft April 3, 2024 20:58
@psychedelicious
Copy link
Collaborator Author

marking draft pending more discussion, good points raised

@lstein
Copy link
Collaborator

lstein commented Apr 12, 2024

Mistakes happen and some users will end up with a model config in the database but missing model files. The system will complain at startup. What is the user to do to remedy the problem?

I would suggest either: 1) adding a UI button "Remove orphan models" or 2) a "Remove orphan models" command line script (accessible through the launcher), which removes the dangling model configs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api python PRs that change python files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: Model database remains stale
5 participants