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

update model_list to allow to set whether to update model #1828

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

LeonZhao28
Copy link
Contributor

add a parameter in the method /controlnet/model_list to allow user to set whether to update model

@huchenlei
Copy link
Collaborator

huchenlei commented Jul 25, 2023

Generally, a GET request should not have side effect on the server. If you want an option to refresh model_list using web API, I would suggest adding a new POST endpoint.

@LeonZhao28
Copy link
Contributor Author

Generally, a GET request should not have side effect on the server. If you want an option to refresh model_list using web API, I would suggest adding a new POST endpoint.

Yes, I agree. But I actually hope to not reload the local model list every time I call this api. The change I pushed is actually to ensure consistency with existing Api usage. From an interface design perspective, it is not reasonable to force a refresh of the local model list. So, do you think it's better to keep the current habit of not changing the small-scale optimization API, or to make it more reasonable but lacking it will affect the usage of the existing API?

@huchenlei
Copy link
Collaborator

Sorry I missed the context. Currently we are doing the refresh everytime the API is called. From that perspective, I think your change is reasonable. We should not break the API consistency without collecting metrics, as users might depend on existing behaviour.

Thanks for spotting the issue and fix it.

@huchenlei huchenlei self-requested a review July 26, 2023 02:51
Copy link
Collaborator

@huchenlei huchenlei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@huchenlei huchenlei merged commit ff7aad9 into Mikubill:main Jul 26, 2023
1 check passed
tiangles pushed a commit to diffus-me/sd-webui-controlnet that referenced this pull request Aug 18, 2023
)

* update model_list to allow to set whether to update model

* update model_list to allow to set whether to update model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants