-
Notifications
You must be signed in to change notification settings - Fork 6k
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
added the download model from url feature #2423
base: main
Are you sure you want to change the base?
added the download model from url feature #2423
Conversation
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.
General feedback: thank you for the implementation, this can be very useful if people don't know the file structure.
Code feedback:
- do not spam code comments for trivial things, as mentioned.
- do not autoformat whole files on existing projects, only your part of the implementation.
- add translations for every outpout, every message, etc. to en.json
- add argument for disabling the whole tab as not everyone wants/needs/allows users to download newc models, such as on hosted environments, as this may cause a huge increase in traffic (=> more costs) and may be abused, especially in a multi-user environment.
Looking forward to your adjustments.
@mashb1t thank you for your feedback if there any other that need to improve feel free to tell me :) |
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.
keep in mind to add the translations to en.json as well as add an argument to completely not render the output for all elements when set.
Argument can be --disable-download-tab
or similar. You can orient yourself on --disable-metadata
+ all occurrences of args_manager.args.disable_metadata in the code
hi! if you can only download from civitAI you can get the filename and folder from the api. civitai provide array of files the first one is always the model file. the api also give you the model type you can select the folder based on the model type. |
@uptotec yes, for that we need to add store feature using civitai api but this is for downloading models directly into the application using url only also i have tested with huggingface and civitai urls and some other it works! without any issues but note that some model may have sign-in access restriction for those we can not download directly just with url alone |
64751d8
to
4820f31
Compare
@mashb1t i have modified according to your suggestion and also we can disable download tab using argument |
@codezeros thanks for the update, much appreciated. Will do the review during this week. |
@mashb1t i thought user can also disable the tab in fooocus itself to simply make the config easier for non-tech users which may help |
…_model_from_url_feature
@@ -433,6 +453,7 @@ def update_history_link(): | |||
disable_seed_increment = gr.Checkbox(label='Disable seed increment', | |||
info='Disable automatic seed increment when image number is > 1.', | |||
value=False) | |||
disable_download_checkbox = gr.Checkbox(label='Disable Download Tab',info='Disable the download tab when clicked',value=modules.config.default_download_tab_checkbox,elem_classes='min_check') |
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.
set interactive to false when arg is set
@@ -541,6 +562,9 @@ def model_refresh_clicked(): | |||
advanced_checkbox.change(lambda x: gr.update(visible=x), advanced_checkbox, advanced_column, | |||
queue=False, show_progress=False) \ | |||
.then(fn=lambda: None, _js='refresh_grid_delayed', queue=False, show_progress=False) | |||
disable_download_checkbox.change(lambda x: gr.update(visible=x), disable_download_checkbox, download_tab) \ |
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.
do not process/output when arg is true
@@ -332,6 +333,25 @@ def update_history_link(): | |||
show_progress=False).then( | |||
lambda: None, _js='()=>{refresh_style_localization();}') | |||
|
|||
if not args_manager.args.disable_download_tab: | |||
with gr.Tab(label='Download',visible=modules.config.default_download_tab_checkbox)as download_tab: |
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.
with gr.Tab(label='Download',visible=modules.config.default_download_tab_checkbox)as download_tab: | |
with gr.Tab(label='Download', visible=modules.config.default_download_tab_checkbox) as download_tab: |
path_controlnet = get_dir_or_set_default('path_controlnet', '../models/controlnet/') | ||
path_clip_vision = get_dir_or_set_default('path_clip_vision', '../models/clip_vision/') | ||
path_fooocus_expansion = get_dir_or_set_default('path_fooocus_expansion', '../models/prompt_expansion/fooocus_expansion') | ||
config_paths = { |
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.
please do it the other way round.
Instead of using an array and then assigning variables with keys it's more robust to assign variables first and then build an array from vars. This eliminates the need for array key string reading.
@@ -318,6 +347,11 @@ def get_config_item_or_set_default(key, default_value, validator, disable_empty_ | |||
default_value=False, | |||
validator=lambda x: isinstance(x, bool) | |||
) | |||
default_download_tab_checkbox = get_config_item_or_set_default( |
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.
please use a more descriptive naming here, e.g. default_show_download_tab
Some personal advice:
|
from torch.hub import download_url_to_file | ||
download_url_to_file(url, cached_file, progress=progress) | ||
except Exception as e: | ||
print(f"Failed to download \"{url}\" to {cached_file}, reason : {e}") |
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.
@codezeros downloading an invalid URL still reports as successful to the frontend despite raising an exception.
This has to be fixed (reverted).
Failed to download "invalid url" to /Volumes/data/private/development/ai/fooocus/models/checkpoints/invalid url, reason : unknown url type: 'invalid url'
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.
@mashb1t thanks for notifying the problem i will look into it and also making some improvements based on previous feedback (in progress)
That's enhancement is great! Thank you @codezeros |
@codezeros |
Models can be downloaded simply by copying and pasting their URLs.