-
Notifications
You must be signed in to change notification settings - Fork 68
[SDK-503] Prevent users from uploading video annotations over the API limit (5000). #1347
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
0206379 to
dfbc2f3
Compare
| data_row_uids, # sample of data row objects | ||
| 5 # priority between 1(Highest) - 5(lowest) | ||
| ) | ||
| labels = [create_label() for _ in range(5000)] |
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.
Are you considering running a full upload of 5,000 annotations for SDK integration testing? This could significantly impact performance and might even result in flakiness. I recall we previously reduced some tests due to similar concerns. Would you consider creating a method or perhaps patching the LABEL_LIMIT to a smaller number, like 2, specifically for integration tests to mitigate these issues?
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'm happy to patch to improve test performance, but I don't think LABEL_LIMIT has any impact on performance here. I did an experiment, results below:
Runtime with LABEL_LIMIT patched to 20 labels:
41.18s call tests/integration/annotation_import/test_bulk_import_request.py::test_below_annotation_limit_on_single_data_row
30.34s call tests/integration/annotation_import/test_bulk_import_request.py::test_above_annotation_limit_on_single_data_row
9.20s setup tests/integration/annotation_import/test_bulk_import_request.py::test_below_annotation_limit_on_single_data_row
5.96s setup tests/integration/annotation_import/test_bulk_import_request.py::test_above_annotation_limit_on_single_data_row
1.04s teardown tests/integration/annotation_import/test_bulk_import_request.py::test_below_annotation_limit_on_single_data_row
0.94s teardown tests/integration/annotation_import/test_bulk_import_request.py::test_above_annotation_limit_on_single_data_row
Runtimes with LABEL_LIMIT not patched:
43.55s call tests/integration/annotation_import/test_bulk_import_request.py::test_below_annotation_limit_on_single_data_row
31.41s call tests/integration/annotation_import/test_bulk_import_request.py::test_above_annotation_limit_on_single_data_row
9.22s setup tests/integration/annotation_import/test_bulk_import_request.py::test_below_annotation_limit_on_single_data_row
6.34s setup tests/integration/annotation_import/test_bulk_import_request.py::test_above_annotation_limit_on_single_data_row
1.13s teardown tests/integration/annotation_import/test_bulk_import_request.py::test_below_annotation_limit_on_single_data_row
1.09s teardown tests/integration/annotation_import/test_bulk_import_request.py::test_above_annotation_limit_on_single_data_row
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'd like to leave it as is, given that patching doesn't have impact on performance (and test results have been consistent in every run, so I don't think flakiness is an issue either).
dfbc2f3 to
d191bfc
Compare
No description provided.