-
Notifications
You must be signed in to change notification settings - Fork 68
improve result fetching experience #593
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
msokoloff1
commented
Jun 3, 2022
- Fetch the result file for the user
- Try to match AnnotationImport.statuses and .errors as close as possible.
fab754d
to
a645024
Compare
5bfa80f
to
e0e8265
Compare
return data.get('error') | ||
if self.status == "FAILED": | ||
result = self._fetch_remote_json() | ||
return result['error'] |
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.
result.get('error')?
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 was thinking that this should never be None since we are specifically checking that the action is a Json import.
labelbox/schema/task.py
Outdated
self.wait_till_done(timeout_seconds=600) | ||
if self.status == "IN_PROGRESS": | ||
raise ValueError( | ||
"Job status still in `IN_PROGRESS`. The result is not available. Increase timeout or contact support." |
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.
Increase timeout or contact support.
since this method is being called from errors
and result
but those don't accept a timeout param, increasing the timeout would be harder for a user. Should we include a timeout param in the other methods, or just ask them to contact support?
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.
Also since this is lru_cache, does that mean that this will cache the first result, and always return the same result for any subsequent calls while a script runs? Would it make more sense since we don't need to evict any items from the cache, to just use @cache instead?
https://docs.python.org/3/library/functools.html#functools.cache
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.
They can wait_till_done
before calling this with a larger timeout. I'll update the message to make this clear. Also the cache does not save the result if an error is thrown.