-
Notifications
You must be signed in to change notification settings - Fork 41
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
feat: add checksum verification for downloaded models #1078
Conversation
Rebased. Ready to be reviewed. |
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.
works as expected (on mac)
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.
PACKAGING-GUIDE.MD should be updated
@jeffmaury updated |
Moved the error displayed on the top as suggested by @axel7083 . TBH i preferred the error on the row as it was in line with desktop but as we rely on the top banner for showing the download in progress it makes sense to show it there. Updated the gif on the first post |
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.
Except the yarn.lock change LGTM, great job
Signed-off-by: lstocchi <[email protected]>
Signed-off-by: lstocchi <[email protected]>
Signed-off-by: lstocchi <[email protected]>
Signed-off-by: lstocchi <[email protected]>
Signed-off-by: lstocchi <[email protected]>
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.
Thanks for all changes, great work! 🚀
@jeffmaury if it's good for you, it can be merged. Give it a look when you have time |
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.
LGTM. Maybe in the documentation we may indicate that the field is optional and should be HEX encoded
Signed-off-by: lstocchi <[email protected]>
What does this PR do?
It display an error if the downloaded model has a different sha than the expected value
Screenshot / video of UI
On the models page
On the recipe page (it still shows the old error mesage with the double
Something went wrong
but it has been cleaned)What issues does this PR fix or reference?
it resolves #977
How to test this PR?