-
Notifications
You must be signed in to change notification settings - Fork 11.3k
Only check frontend package if using default frontend #7179
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
Conversation
robinjhuang
left a comment
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.
Tested and works on Desktop
| with open(req_path, "r", encoding="utf-8") as f: | ||
| required_frontend = parse_version(f.readline().split("=")[-1]) | ||
| if frontend_version < required_frontend: | ||
| logging.warning("________________________________________________________________________\nWARNING WARNING WARNING WARNING WARNING\n\nInstalled frontend version {} is lower than the recommended version {}.\n\n{}\n________________________________________________________________________".format('.'.join(map(str, frontend_version)), '.'.join(map(str, required_frontend)), frontend_install_warning_message())) |
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 warning needs to show up when the comfyui_frontend_package.version doesn't exist, it is for those who update and run comfyui manually in the console so it needs to be visible without scrolling up after launching comfyui.
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.
version("comfyui-frontend-package") will get the package version before when we expose it in 1.11.8.
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.
I can delay the execution of check_version to the original spot, but it breaks the overall execution flow.
Should we make a standard mechanism to delay every warning during startup after the server start log?
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.
yeah we should or else people will probably ignore them.
Ref: #7021 (comment)
The desktop app is also affected as it is currently shipping the frontend package twice, increasing the oveall disk space consumption for users.
This PR bypass the import of built-in frontend package and frontend version check if the user launches with
--front-end-rootor--front-end-versionargument to use alternative frontend implementation.