Skip to content
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

Maintains compatibility with CLI return, broken in PR #266 #309

Closed

Conversation

lrosemberg
Copy link
Contributor

@lrosemberg lrosemberg commented Aug 18, 2024

Description

In PR #266 the raised errors changed how the CLI response is sent.

Said that, what this PR does?

  • Create a new custom exceptions to handle correctly in CLI dataset upload
  • Fix the bug introduced in @266 that fails to parse some image upload responses (more info here: fix: raise UploadErrors for images and annotations #266 (comment))
  • Isolates the error handling to _log_img_upload_err that earlier is shared with _log_img_upload
  • Create some test cases to validate the project.upload

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How has this change been tested, please provide a testcase or example of how you tested the change?

  • Created some tests in the code to validate the project.upload bugfix
  • Downloaded a dataset, edited a annotation to fail, and uploaded using the CLI, results below:

CleanShot 2024-08-18 at 20 09 00@2x

CleanShot 2024-08-18 at 20 12 02@2x

@lrosemberg lrosemberg marked this pull request as ready for review August 18, 2024 23:17
Comment on lines -572 to +584
dict_part = str(error).split(": ", 2)[2]
error_str = str(error)
start_idx = error_str.index("{")
end_idx = error_str.rindex("}") + 1
dict_part = error_str[start_idx:end_idx]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the bug reported here:

#266 (comment)

@lrosemberg lrosemberg closed this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant