-
Notifications
You must be signed in to change notification settings - Fork 12
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
Report cloud upload errors in a more structured way #582
Conversation
encord/orm/dataset.py
Outdated
@@ -1008,6 +1026,9 @@ class DatasetDataLongPolling(BaseDTO): | |||
errors: List[str] | |||
"""Stringified list of exceptions.""" | |||
|
|||
data_units_errors: Optional[List[DataUnitError]] = None |
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 is optional mostly "just in case"; the "case" being "we released the SDK but had to roll back the BE so the field is not returned anymore".
We can probably make this type a notch nicer if we wait with merging this until after the BE is firmly out there.
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, i'd vote for this option!
Not sure why or how has this ended up "closed"; we just need to wait until after 2024-04-24 release before we merge and release this... |
Introduction and Explanation
Right now, we have
.errors
in the upload result object, but it's all just strings. Return a bit more data so that at least the failed URLs are machine readable.JIRA
Fixes https://linear.app/encord/issue/EC-3107/report-upload-errors-to-the-sdk-in-a-reasonably-machine-readable
Documentation
Some dosctrings are there
Tests
Coming together with the BE implementation here: https://github.com/encord-team/cord-backend/pull/3206
Known issues
We'll need to do the same on the
Storage
side; will have a follow-up probably next week.