From 0fb7bbe58ff44f239a0dbe4d79ac13cd85f789ae Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 10:24:21 +0100 Subject: [PATCH 01/19] Fix error tests --- labelbox/client.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/labelbox/client.py b/labelbox/client.py index c8ca5b1d4..492165cc2 100644 --- a/labelbox/client.py +++ b/labelbox/client.py @@ -186,11 +186,26 @@ def check_errors(keywords, *path): if response_msg.startswith("You have exceeded"): raise labelbox.exceptions.ApiLimitError(response_msg) - prisma_error = check_errors(["INTERNAL_SERVER_ERROR"], "extensions", - "code") - if prisma_error: - raise labelbox.exceptions.InternalServerError( - prisma_error["message"]) + resource_not_found_error = check_errors( + ["RESOURCE_NOT_FOUND"], "extensions", "exception", "code") + if resource_not_found_error is not None: + # Return None and let the caller methods raise an exception + # as they already know which resource type and ID was requested + return None + + # A lot of different error situations are now labeled serverside + # as INTERNAL_SERVER_ERROR, when they are actually client errors. + # TODO: fix this in the server API + internal_server_error = check_errors(["INTERNAL_SERVER_ERROR"], + "extensions", "code") + if internal_server_error is not None: + message = internal_server_error.get("message") + + if message.startswith("Syntax Error"): + raise labelbox.exceptions.InvalidQueryError(message) + + else: + raise labelbox.exceptions.InternalServerError(message) if len(errors) > 0: logger.warning("Unparsed errors on query execution: %r", errors) @@ -297,7 +312,7 @@ def _get_single(self, db_object_type, uid): """ query_str, params = query.get_single(db_object_type, uid) res = self.execute(query_str, params) - res = res[utils.camel_case(db_object_type.type_name())] + res = res and res.get(utils.camel_case(db_object_type.type_name())) if res is None: raise labelbox.exceptions.ResourceNotFoundError( db_object_type, params) From e4de4fecb6998d5985c760e73a95660ed2841a68 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 11:09:38 +0100 Subject: [PATCH 02/19] Improve DbObject equality testing --- labelbox/orm/db_object.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/labelbox/orm/db_object.py b/labelbox/orm/db_object.py index d6453f64f..6f0a69abd 100644 --- a/labelbox/orm/db_object.py +++ b/labelbox/orm/db_object.py @@ -82,7 +82,9 @@ def __str__(self): return "<%s %s>" % (self.type_name().split(".")[-1], attribute_values) def __eq__(self, other): - return self.type_name() == other.type_name() and self.uid == other.uid + return (isinstance(other, DbObject) and + self.type_name() == other.type_name() and + self.uid == other.uid) def __hash__(self): return 7541 * hash(self.type_name()) + hash(self.uid) From 5389da001aeb053d9804c941db1c6314dacbcb53 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 11:10:35 +0100 Subject: [PATCH 03/19] Make to-one relationship fetching more robust --- labelbox/orm/db_object.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/labelbox/orm/db_object.py b/labelbox/orm/db_object.py index 6f0a69abd..b68bc7ad9 100644 --- a/labelbox/orm/db_object.py +++ b/labelbox/orm/db_object.py @@ -154,8 +154,9 @@ def _to_one(self): query_string, params = query.relationship(self.source, rel, None, None) result = self.source.client.execute(query_string, params) - result = result[utils.camel_case(type(self.source).type_name())] - result = result[rel.graphql_name] + result = result and result.get( + utils.camel_case(type(self.source).type_name())) + result = result and result.get(rel.graphql_name) if result is None: return None return rel.destination_type(self.source.client, result) From 085c6f37defa1128cbeee9cea6c986e58acf3d25 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 11:23:02 +0100 Subject: [PATCH 04/19] Fix Project.labels ordering --- labelbox/schema/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/labelbox/schema/project.py b/labelbox/schema/project.py index 411ec2258..b8afa1ec9 100644 --- a/labelbox/schema/project.py +++ b/labelbox/schema/project.py @@ -134,7 +134,7 @@ def labels(self, datasets=None, order_by=None): id_param = "projectId" query_str = """query GetProjectLabelsPyApi($%s: ID!) {project (where: {id: $%s}) - {labels (skip: %%d first: %%d%s%s) {%s}}}""" % ( + {labels (skip: %%d first: %%d %s %s) {%s}}}""" % ( id_param, id_param, where, order_by_str, query.results_query_part(Label)) From e03ffd76ae521da2ea1f3c9b4fdc4e249c695bd0 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 11:29:27 +0100 Subject: [PATCH 05/19] Mark tests as xfail (server-side issues) or skip (with added TODO) to fix --- tests/integration/test_client_errors.py | 2 +- tests/integration/test_data_rows.py | 4 +++- tests/integration/test_data_upload.py | 6 +++++- tests/integration/test_label.py | 1 + tests/integration/test_sorting.py | 8 ++++---- tests/integration/test_webhook.py | 4 ++++ 6 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_client_errors.py b/tests/integration/test_client_errors.py index d1dacd5fe..b5cccc05e 100644 --- a/tests/integration/test_client_errors.py +++ b/tests/integration/test_client_errors.py @@ -103,7 +103,7 @@ def test_invalid_attribute_error(client, rand_gen): project.delete() -@pytest.mark.skip +@pytest.mark.slow def test_api_limit_error(client, rand_gen): project_id = client.create_project(name=rand_gen(str)).uid diff --git a/tests/integration/test_data_rows.py b/tests/integration/test_data_rows.py index 0b4563f0d..bb5e30a9a 100644 --- a/tests/integration/test_data_rows.py +++ b/tests/integration/test_data_rows.py @@ -49,6 +49,8 @@ def test_data_row_bulk_creation(dataset, rand_gen): data_rows[0].delete() +@pytest.mark.slow +def test_data_row_large_bulk_creation(dataset, rand_gen): # Do a longer task and expect it not to be complete immediately with NamedTemporaryFile() as fp: fp.write("Test data".encode()) @@ -62,7 +64,7 @@ def test_data_row_bulk_creation(dataset, rand_gen): data_rows = len(list(dataset.data_rows())) == 5003 -@pytest.mark.skip +@pytest.mark.xfail(reason="DataRow.dataset() relationship not set") def test_data_row_single_creation(dataset, rand_gen): client = dataset.client assert len(list(dataset.data_rows())) == 0 diff --git a/tests/integration/test_data_upload.py b/tests/integration/test_data_upload.py index 6d2226522..60ce78272 100644 --- a/tests/integration/test_data_upload.py +++ b/tests/integration/test_data_upload.py @@ -1,7 +1,11 @@ +import pytest import requests -def test_file_uplad(client, rand_gen): +# TODO it seems that at some point Google Storage (gs prefix) started being +# returned, and we can't just download those with requests. Fix this +@pytest.mark.skip +def test_file_upload(client, rand_gen): data = rand_gen(str) url = client.upload_data(data.encode()) assert requests.get(url).text == data diff --git a/tests/integration/test_label.py b/tests/integration/test_label.py index e2319fe4a..8399f1334 100644 --- a/tests/integration/test_label.py +++ b/tests/integration/test_label.py @@ -30,6 +30,7 @@ def test_labels(label_pack): assert list(data_row.labels()) == [] +# TODO check if this is supported or not @pytest.mark.skip def test_label_export(label_pack): project, dataset, data_row, label = label_pack diff --git a/tests/integration/test_sorting.py b/tests/integration/test_sorting.py index a10b32a43..07dba6128 100644 --- a/tests/integration/test_sorting.py +++ b/tests/integration/test_sorting.py @@ -3,7 +3,8 @@ from labelbox import Project -@pytest.mark.skip +@pytest.mark.xfail(reason="Relationship sorting not implemented correctly " + "on the server-side") def test_relationship_sorting(client): a = client.create_project(name="a", description="b") b = client.create_project(name="b", description="c") @@ -29,7 +30,6 @@ def get(order_by): c.delete() +@pytest.mark.xfail(reason="Sorting not supported on top-level fetches") def test_top_level_sorting(client): - # TODO support sorting on top-level fetches - with pytest.raises(TypeError): - client.get_projects(order_by=Project.name.asc) + client.get_projects(order_by=Project.name.asc) diff --git a/tests/integration/test_webhook.py b/tests/integration/test_webhook.py index 194760b3c..66fb86312 100644 --- a/tests/integration/test_webhook.py +++ b/tests/integration/test_webhook.py @@ -1,6 +1,10 @@ +import pytest + from labelbox import Webhook +# TODO investigate why this fails +@pytest.mark.skip def test_webhook_create_update(project, rand_gen): client = project.client url = "https:/" + rand_gen(str) From de57a9ed2f5e75763df035e9a09c0dea6fceef1a Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 11:31:49 +0100 Subject: [PATCH 06/19] Skip slow tests when running tox --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 0ff6b5bca..8bde69205 100644 --- a/tox.ini +++ b/tox.ini @@ -8,4 +8,4 @@ deps = -rrequirements.txt pytest passenv = LABELBOX_TEST_API_KEY_PROD LABELBOX_TEST_API_KEY_STAGING LABELBOX_TEST_ENVIRON -commands = pytest {posargs} +commands = pytest -m "not slow" {posargs} From 7f90eabeab3a7e9a3e161958b4a5997841fe940c Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 11:48:21 +0100 Subject: [PATCH 07/19] Delete test_logger as it tests Pytest and not Labelbox --- tests/integration/test_logger.py | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 tests/integration/test_logger.py diff --git a/tests/integration/test_logger.py b/tests/integration/test_logger.py deleted file mode 100644 index c8b9dcf8b..000000000 --- a/tests/integration/test_logger.py +++ /dev/null @@ -1,20 +0,0 @@ -from labelbox import Client -import pytest -import logging - - -def test_client_log(caplog, project): - """ - This file tests that the logger will properly output to the console after updating logging level - - The default level is set to WARNING - - There is an expected output after setting logging level to DEBUG - """ - - project.export_labels() - assert '' == caplog.text - - with caplog.at_level(logging.DEBUG): - project.export_labels() - assert "label export, waiting for server..." in caplog.text From 35a2d77baa1206e1d249e2756e9cf29c5178494b Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 12:15:55 +0100 Subject: [PATCH 08/19] Update CHANGELOG --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e346f7c..148d2e103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,11 @@ # Changelog + +## In progress +### Fix +* Custom queries with bad syntax now raise adequate exceptions (InvalidQuery) +* Comparing a Labelbox object (e.g. Project) to None doesn't raise an exception +* Adding `order_by` to `Project.labels` doesn't raise an exception + ## Version 2.4.9 (2020-11-09) ### Fix * 2.4.8 was broken for > Python 3.6 From 256de7ab8a24cbc2181b8bc4566fbedc022284d5 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 12:18:11 +0100 Subject: [PATCH 09/19] Perform YAPF --- docs/source/conf.py | 6 +----- labelbox/client.py | 5 +++-- labelbox/orm/db_object.py | 3 +-- tests/integration/test_data_rows.py | 1 + 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/docs/source/conf.py b/docs/source/conf.py index 131ae59bc..df6ce2145 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -14,7 +14,6 @@ import sys sys.path.insert(0, os.path.abspath('../..')) - # -- Project information ----------------------------------------------------- project = 'Labelbox Python API reference' @@ -29,9 +28,7 @@ # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom # ones. extensions = [ - 'sphinx.ext.autodoc', - 'sphinx.ext.viewcode', - 'sphinxcontrib.napoleon' + 'sphinx.ext.autodoc', 'sphinx.ext.viewcode', 'sphinxcontrib.napoleon' ] # Add any paths that contain templates here, relative to this directory. @@ -42,7 +39,6 @@ # This pattern also affects html_static_path and html_extra_path. exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store'] - # -- Options for HTML output ------------------------------------------------- # The theme to use for HTML and HTML Help pages. See the documentation for diff --git a/labelbox/client.py b/labelbox/client.py index 492165cc2..dd64e0eda 100644 --- a/labelbox/client.py +++ b/labelbox/client.py @@ -186,8 +186,9 @@ def check_errors(keywords, *path): if response_msg.startswith("You have exceeded"): raise labelbox.exceptions.ApiLimitError(response_msg) - resource_not_found_error = check_errors( - ["RESOURCE_NOT_FOUND"], "extensions", "exception", "code") + resource_not_found_error = check_errors(["RESOURCE_NOT_FOUND"], + "extensions", "exception", + "code") if resource_not_found_error is not None: # Return None and let the caller methods raise an exception # as they already know which resource type and ID was requested diff --git a/labelbox/orm/db_object.py b/labelbox/orm/db_object.py index b68bc7ad9..45feedd9f 100644 --- a/labelbox/orm/db_object.py +++ b/labelbox/orm/db_object.py @@ -83,8 +83,7 @@ def __str__(self): def __eq__(self, other): return (isinstance(other, DbObject) and - self.type_name() == other.type_name() and - self.uid == other.uid) + self.type_name() == other.type_name() and self.uid == other.uid) def __hash__(self): return 7541 * hash(self.type_name()) + hash(self.uid) diff --git a/tests/integration/test_data_rows.py b/tests/integration/test_data_rows.py index bb5e30a9a..59be6309b 100644 --- a/tests/integration/test_data_rows.py +++ b/tests/integration/test_data_rows.py @@ -49,6 +49,7 @@ def test_data_row_bulk_creation(dataset, rand_gen): data_rows[0].delete() + @pytest.mark.slow def test_data_row_large_bulk_creation(dataset, rand_gen): # Do a longer task and expect it not to be complete immediately From 2ce1532c854358f165c7ba4d30aca93c4dcc5eee Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 12:37:19 +0100 Subject: [PATCH 10/19] Add pytest as GH action dep --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index a070e6ab0..b3b8d97dd 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -58,7 +58,7 @@ jobs: mypy -p labelbox --pretty --show-error-codes - name: Install package and test dependencies run: | - pip install tox==3.18.1 + pip install tox==3.18.1 pytest==5.3.1 # TODO: replace tox.ini with what the Makefile does # to make sure local testing is From a6b6363b08860079c1e7e78cfda1142db769741e Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 15:30:27 +0100 Subject: [PATCH 11/19] Replace pytest invocation with tox in GitHub action --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index b3b8d97dd..af40e1725 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -73,4 +73,4 @@ jobs: # randall+staging-python@labelbox.com LABELBOX_TEST_API_KEY_STAGING: ${{ secrets.STAGING_LABELBOX_API_KEY }} run: | - pytest -svv + tox From 4f90219768c1bc87c9f35505ab4be6ac1209ab58 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Wed, 20 Jan 2021 16:42:10 +0100 Subject: [PATCH 12/19] Add all Py versions to GitHub build workflow --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index af40e1725..35597f62b 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -15,7 +15,7 @@ jobs: max-parallel: 1 matrix: # TODO: unlock parallel testing by using more API keys - python-version: [3.6] + python-version: [3.6, 3.7, 3.8] steps: From 51ccc24e546e435da2fbe9010fa5d5b348c9027f Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Thu, 21 Jan 2021 08:54:10 +0100 Subject: [PATCH 13/19] Fix tox invocation in GitHub workflow --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 35597f62b..3fa1c5c0e 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -73,4 +73,4 @@ jobs: # randall+staging-python@labelbox.com LABELBOX_TEST_API_KEY_STAGING: ${{ secrets.STAGING_LABELBOX_API_KEY }} run: | - tox + tox -e py From b733f15e4ebd80322bd2f8195e699f54106528e3 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Thu, 21 Jan 2021 09:16:09 +0100 Subject: [PATCH 14/19] Remove unnecessary dependency in GH workflow --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 3fa1c5c0e..e8d832985 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -58,7 +58,7 @@ jobs: mypy -p labelbox --pretty --show-error-codes - name: Install package and test dependencies run: | - pip install tox==3.18.1 pytest==5.3.1 + pip install tox==3.18.1 # TODO: replace tox.ini with what the Makefile does # to make sure local testing is From 15a059352dd881cb8382063591b969a2784fef1e Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Fri, 22 Jan 2021 14:35:35 +0100 Subject: [PATCH 15/19] Change pytest flags in GitHub build --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index e8d832985..49b92d33c 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -73,4 +73,4 @@ jobs: # randall+staging-python@labelbox.com LABELBOX_TEST_API_KEY_STAGING: ${{ secrets.STAGING_LABELBOX_API_KEY }} run: | - tox -e py + tox -e py -- -svv From ca4e2a24ca607a1102492a0d9765c11ddbebab91 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Mon, 25 Jan 2021 09:13:47 +0100 Subject: [PATCH 16/19] Remove 'not slow' from tox pytest invocation --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 8bde69205..0ff6b5bca 100644 --- a/tox.ini +++ b/tox.ini @@ -8,4 +8,4 @@ deps = -rrequirements.txt pytest passenv = LABELBOX_TEST_API_KEY_PROD LABELBOX_TEST_API_KEY_STAGING LABELBOX_TEST_ENVIRON -commands = pytest -m "not slow" {posargs} +commands = pytest {posargs} From b59c494db508ab0bb8b292ee209d067b70f5576f Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Mon, 25 Jan 2021 09:55:36 +0100 Subject: [PATCH 17/19] Treat 'upstream connect error' as something to retry after --- labelbox/client.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/labelbox/client.py b/labelbox/client.py index dd64e0eda..14384da84 100644 --- a/labelbox/client.py +++ b/labelbox/client.py @@ -139,6 +139,10 @@ def convert_value(value): error_502 = '502 Bad Gateway' if error_502 in response.text: raise labelbox.exceptions.InternalServerError(error_502) + if "upstream connect error or disconnect/reset before headers" \ + in response.text: + raise labelbox.exceptions.InternalServerError( + "Connection reset") raise labelbox.exceptions.LabelboxError( "Failed to parse response as JSON: %s" % response.text) From 9e04b133ba40ffe5b30b367a30056adce127e212 Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Mon, 25 Jan 2021 09:56:06 +0100 Subject: [PATCH 18/19] Increase number of requests in the API call limit test --- tests/integration/test_client_errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_client_errors.py b/tests/integration/test_client_errors.py index b5cccc05e..d44c10be2 100644 --- a/tests/integration/test_client_errors.py +++ b/tests/integration/test_client_errors.py @@ -114,7 +114,7 @@ def get(arg): return e with Pool(300) as pool: - results = pool.map(get, list(range(1000))) + results = pool.map(get, list(range(2000))) assert labelbox.exceptions.ApiLimitError in {type(r) for r in results} From 1d89f59adbaa175a81f4134b4d376ff85710b7cc Mon Sep 17 00:00:00 2001 From: Florijan Stamenkovic Date: Mon, 25 Jan 2021 10:43:48 +0100 Subject: [PATCH 19/19] Skipping API limit test due to flakyness --- tests/integration/test_client_errors.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_client_errors.py b/tests/integration/test_client_errors.py index d44c10be2..9fad57ffb 100644 --- a/tests/integration/test_client_errors.py +++ b/tests/integration/test_client_errors.py @@ -104,6 +104,8 @@ def test_invalid_attribute_error(client, rand_gen): @pytest.mark.slow +# TODO improve consistency +@pytest.mark.skip(reason="Inconsistent test") def test_api_limit_error(client, rand_gen): project_id = client.create_project(name=rand_gen(str)).uid