-
-
Notifications
You must be signed in to change notification settings - Fork 133
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
Feature/text model metadata api #456
base: main
Are you sure you want to change the base?
Feature/text model metadata api #456
Conversation
Add `metadata` arg to /v2/status/models endpoint to request model metadata for available models Refactor model_reference.py
0032bd6
to
173ce91
Compare
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.
Some reorg changes, but I don't see anything seriously wrong. Can you also see if you can test this in python 3.9 to see if the added typing throws an exception?
@@ -1278,6 +1279,232 @@ def __init__(self, api): | |||
), | |||
}, | |||
) | |||
self.response_model_known_model_md = api.model( |
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.
There's a separate file for image and text worker models. kobold_v2.py and stable_v2.py. You need to store the type-specific models in the right file to keep things somewhat organized.
Common data in both types should stay in v2.py and then use model inheritance to extend for each type.
help=( | ||
"If 'known', only show stats for known models in the model reference. " | ||
"If 'custom' only show stats for custom models. " | ||
"If 'all' shows stats for all models." | ||
), | ||
location="args", | ||
) | ||
get_parser.add_argument( |
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.
bool arguments in metadata are a bit buggy. A ?metadata=False
query is treated as True
, sadly, because it stupidly casts the string into a bool. To work around this, I suggest no default and then treat None as "False".
To properly work around this requires a bit of a hack by looking into the request query directly
Sadly the library we're using doesn't seem maintained well anymore :(
@@ -265,7 +265,7 @@ def worker_exists(worker_id): | |||
return wc | |||
|
|||
|
|||
def get_available_models(filter_model_name: str = None): | |||
def get_available_models(filter_model_name: str = None) -> list[dict]: |
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.
Horde is still largely running in Python 3.9 so typing is not well supported there. I can't remember if this declaration works in 3.9
metadata: KnownModelRef | ||
|
||
|
||
def known_models(model_type: str = None, filter_model_name: str = None, exact: bool = None) -> list[KnownModelsReturn]: |
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.
This should go inside the model_reference.py
instead of at the root of the API file.
ref_json = DEFAULT_HORDE_IMAGE_COMPVIS_REFERENCE | ||
if datetime.now(timezone.utc) <= datetime(2024, 9, 30, tzinfo=timezone.utc): | ||
# Flux Beta | ||
# I don't understand how this hack works, but perhaps HORDE_IMAGE_COMPVIS_REFERENCE is unset in prod |
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, it's unset in prod. This env var is to allow private hordes to use their own references. You can delete this comment
This is the second incremental PR for #452
I appreciate your review and am happy to revise according to your feedback.
The upshot of this PR would be that model reference data being served from the API is far more rigorously structured than the data being consumed from the json definition files, and somewhat more structured than the data being passed around internally. It would be possible later to extend typing and validation to the point of ingestion, or move the source of truth from JSON into the database. Accuracy and ease of maintenance is important for ongoing viability of #452.
Add
metadata
arg to /v2/status/models endpoint to request model metadata for available modelsAdd list and detail endpoints for known model metadata at /v2/knownmodels /v2/knownmodels/string:model_name
Refactor model_reference.py