-
-
Notifications
You must be signed in to change notification settings - Fork 498
Fsspec async #1111
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
Fsspec async #1111
Conversation
…transformerlab-app into add/fsspec-async
…transformerlab-app into add/fsspec-async
| # Update final progress and success status | ||
| self.progress_update(progress_end) | ||
|
|
||
| job_data = self.job.get_json_data() |
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 didn't hit this while testing, but don't the three get_json_data calls have to be run async?
Side note tangential to this PR: I don't actually get why it does the first one twice with different strings and then call get_json data a third 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.
Okay I made it async.
It is present thrice because I had caching issues with some jobs that finished earlier and if you do get json data while the job just saved the json and an update is running, it would just directly return empty json and then the new update would replace job data causing a lot of issues. Calling get json data buffers it a bit and fixed the issue for me
deep1401
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.
Depends on transformerlab/transformerlab-inference#29
dadmobile
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.
Marking as approved so we can merge when ready. We have failing tests but that's because of the inference version. Our tests were good before that change so we should just check after we release that!
deep1401
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.
All tests passed!
Merging now
No description provided.