-
Notifications
You must be signed in to change notification settings - Fork 9.2k
Added parameter required_frontend_version in the /system_stats API response #8875
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
base: master
Are you sure you want to change the base?
Added parameter required_frontend_version in the /system_stats API response #8875
Conversation
|
||
def test_get_frontend_version(): | ||
#Arrange | ||
expected_version = "1.23.4" |
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 think this will fail once the frontend is updated again. To test this, we could mock something, or write to requirements.txt (ensuring we cleanup the side effect after test is done).
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 made the changes in 56981f5. Apologies for the mistake.
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.
@christian-byrne Please let me know if the changes in this commit 56981f5 align with your expectations. I’m open to making any further changes if needed. Thank you.
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 looks good. I'm going to test today and try to get both PRs merged today or tomorrow.
Can you apply ruff
formatting while I do that?
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.
@christian-byrne Corrected the files to comply with the ruff formatting in de49e6e. Thank you for the update.
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.
@christian-byrne Wanted to check if the latest commit de49e6e solves the ruff formatting error. Please let me know if there are anymore workflow errors, happy to make further corrections if needed.
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.
Looks good, all tests passing!
Added parameter required_frontend_version in the /system_stats API response
comfyui-frontend-package
parameter from the requirements.txt file to get the required frontend version for the backend and added that as an additional parameter in the response of /system_stats API.