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

(fix): load_extra_path_config: relative path not converted to a full path #6395

Conversation

bigcat88
Copy link
Contributor

@bigcat88 bigcat88 commented Jan 8, 2025

No breaking changes, just a small fix + more tests.

  1. Added conversion of base_path to absolute path if it is relative, for correct detection of duplicate paths (and the add_model_folder_path function accepts full_folder_path: str, so it is logical to do so)
  2. Added 3 tests for all cases of extra_model_paths.yaml: when base_path is relative, when it is absolute and when it is not there at all.
  3. Fixed old tests "load_extra_path_config", added OS detection, because the path C:/Users/TestUser/AppData/Roaming/ on Linux is considered relative.

@bigcat88
Copy link
Contributor Author

@robinjhuang I see from the gitlog that you wrote tests in this file, can you review the PR?

p.s.: I don't know why monkeypatch.setattr is used instead of the usual file writing to the temp directory (which will make the tests more readable in my opinion) - if necessary, I can rewrite the tests for writing files to the temp directory.

@comfyanonymous comfyanonymous merged commit b9d9bcb into comfyanonymous:master Jan 12, 2025
5 checks passed
@bigcat88 bigcat88 deleted the fix/extra_config/relative-base-path branch January 12, 2025 05:22
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