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.

Sign up for GitHub

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

fix cache config error #3138

Closed
wants to merge 1 commit into from
Closed

fix cache config error #3138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion TTS/utils/manage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,10 @@ def create_dir_and_download_model(self, model_name, model_item, output_path):

def check_if_configs_are_equal(self, model_name, model_item, output_path):
with fsspec.open(self._find_files(output_path)[1], "r", encoding="utf-8") as f:
config_local = json.load(f)
try:
config_local = json.load(f)
except:
Copy link
Contributor

Choose a reason for hiding this comment

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

A bare except: is not a good idea. For instance, a KeyboardInterrupt will be caught by it.

Instead, catch the particular exception that json.load() would raise for an empty file (json.JSONDecodeError).

Copy link
Author

Choose a reason for hiding this comment

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

Actually IMO for this case a bare except is exactly what is needed, if you consider what problem this commit is solving.

The problem here is that it completely avoids downloading anything if something went wrong. The worst case scenario here is it re-downloads the files. It doesn't matter what goes wrong during the json.load, if something unexpected happens, it should just redownload the files.

Copy link
Member

Choose a reason for hiding this comment

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

IT should avoid downloading anything if something is wrong. Even if you download the remaining files, the model most prob would not work. We should debug why the files empty and fix that. Any ideas ?

Copy link
Contributor

Choose a reason for hiding this comment

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

A bare except: is never a good idea, though. except Exception: maybe, if you really need to catch everything, but except:, never.

Copy link
Member

Choose a reason for hiding this comment

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

+1 for that

config_local = None
remote_url = None
for url in model_item["hf_url"]:
if "config.json" in url:
Expand Down
Loading