Fix minor bugs and tidy tests up (#98)

Author: Florijan Stamenković

.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:
 
@@ -73,4 +73,4 @@ jobs:
         # randall+staging-python@labelbox.com
         LABELBOX_TEST_API_KEY_STAGING: ${{ secrets.STAGING_LABELBOX_API_KEY }}
       run: |
-        pytest -svv
+        tox -e py -- -svv

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

docs/source/conf.py

@@ -14,7 +14,6 @@
 import sys
 sys.path.insert(0, os.path.abspath('../..'))
 
-
 # -- Project information -----------------------------------------------------
 
 project = 'Labelbox Python API reference'
@@ -28,9 +27,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.
@@ -41,7 +38,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

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)
 
@@ -186,11 +190,27 @@ 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 +317,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)

labelbox/orm/db_object.py

@@ -82,7 +82,8 @@ 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)
@@ -152,8 +153,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)

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))
 

tests/integration/test_client_errors.py

@@ -103,7 +103,9 @@ def test_invalid_attribute_error(client, rand_gen):
     project.delete()
 
 
-@pytest.mark.skip
+@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
 
@@ -114,7 +116,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}
 

tests/integration/test_data_rows.py

@@ -49,6 +49,9 @@ 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 +65,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

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

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