address comments

Author: Coniferish

_test_unstructured_client/test__decorators.py

@@ -9,16 +9,18 @@
 
 
 @pytest.mark.parametrize(
-    ("server_url"),
+    "server_url",
     [
-        ("https://unstructured-000mock.api.unstructuredapp.io"),  # correct url
-        ("unstructured-000mock.api.unstructuredapp.io"),
-        ("http://unstructured-000mock.api.unstructuredapp.io/general/v0/general"),
-        ("https://unstructured-000mock.api.unstructuredapp.io/general/v0/general"),
-        ("unstructured-000mock.api.unstructuredapp.io/general/v0/general"),
+        # -- well-formed url --
+        "https://unstructured-000mock.api.unstructuredapp.io",
+        # -- common malformed urls --
+        "unstructured-000mock.api.unstructuredapp.io",
+        "http://unstructured-000mock.api.unstructuredapp.io/general/v0/general",
+        "https://unstructured-000mock.api.unstructuredapp.io/general/v0/general",
+        "unstructured-000mock.api.unstructuredapp.io/general/v0/general",
     ],
 )
-def test_clean_server_url_on_paid_api_url(server_url: str):
+def test_clean_server_url_fixes_malformed_paid_api_url(server_url: str):
     client = UnstructuredClient(
         server_url=server_url,
         api_key_auth=FAKE_KEY,
@@ -30,38 +32,51 @@ def test_clean_server_url_on_paid_api_url(server_url: str):
 
 
 @pytest.mark.parametrize(
-    ("server_url"),
+    "server_url",
     [
-        ("http://localhost:8000"),  # correct url
-        ("localhost:8000"),
-        ("localhost:8000/general/v0/general"),
-        ("http://localhost:8000/general/v0/general"),
+        # -- well-formed url --
+        "http://localhost:8000",
+        # -- common malformed urls --
+        "localhost:8000",
+        "localhost:8000/general/v0/general",
+        "http://localhost:8000/general/v0/general",
     ],
 )
-def test_clean_server_url_on_localhost(server_url: str):
+def test_clean_server_url_fixes_malformed_localhost_url(server_url: str):
     client = UnstructuredClient(
         server_url=server_url,
         api_key_auth=FAKE_KEY,
     )
     assert client.general.sdk_configuration.server_url == "http://localhost:8000"
 
 
-def test_clean_server_url_on_empty_string():
+def test_clean_server_url_returns_empty_string_given_empty_string():
     client = UnstructuredClient(
         server_url="",
         api_key_auth=FAKE_KEY,
     )
     assert client.general.sdk_configuration.server_url == ""
 
 
+def test_clean_server_url_returns_None_given_no_server_url():
+    client = UnstructuredClient(
+        api_key_auth=FAKE_KEY,
+    )
+    assert client.general.sdk_configuration.server_url == None
+
+
 @pytest.mark.parametrize(
-    ("server_url"),
+    "server_url",
     [
-        ("https://unstructured-000mock.api.unstructuredapp.io"),
-        ("unstructured-000mock.api.unstructuredapp.io/general/v0/general"),
+        # -- well-formed url --
+        "https://unstructured-000mock.api.unstructuredapp.io",
+        # -- malformed url --
+        "unstructured-000mock.api.unstructuredapp.io/general/v0/general",
     ],
 )
-def test_clean_server_url_with_positional_arguments(server_url: str):
+def test_clean_server_url_fixes_malformed_urls_with_positional_arguments(
+    server_url: str,
+):
     client = UnstructuredClient(
         FAKE_KEY,
         "",
@@ -73,26 +88,26 @@ def test_clean_server_url_with_positional_arguments(server_url: str):
     )
 
 
-def test_suggest_defining_url_if_401():
-    with pytest.warns(UserWarning):
-
-        client = UnstructuredClient(
-            api_key_auth=FAKE_KEY,
-        )
-
-        filename = "_sample_docs/layout-parser-paper-fast.pdf"
+def test_suggest_defining_url_issues_a_warning_on_a_401():
+    client = UnstructuredClient(
+        api_key_auth=FAKE_KEY,
+    )
 
-        with open(filename, "rb") as f:
-            files = shared.Files(
-                content=f.read(),
-                file_name=filename,
-            )
+    filename = "_sample_docs/layout-parser-paper-fast.pdf"
 
-        req = shared.PartitionParameters(
-            files=files,
+    with open(filename, "rb") as f:
+        files = shared.Files(
+            content=f.read(),
+            file_name=filename,
         )
 
-        try:
+    req = shared.PartitionParameters(
+        files=files,
+    )
+    
+    with pytest.raises(SDKError, match="API error occurred: Status 401"):
+        with pytest.warns(
+            UserWarning,
+            match="If intending to use the paid API, please define `server_url` in your request.",
+        ):
             client.general.partition(req)
-        except SDKError as e:
-            print(e)

src/unstructured_client/utils/_decorators.py

@@ -16,6 +16,13 @@
 
 
 def clean_server_url(func: Callable[_P, None]) -> Callable[_P, None]:
+    """A decorator for fixing common types of malformed 'server_url' arguments.
+
+    This decorator addresses the common problem of users omitting or using the wrong url scheme and/or
+    adding the '/general/v0/general' path to the 'server_url'.
+    The decorator should be manually applied to the __init__ method of UnstructuredClient after merging
+    a PR from Speakeasy.
+    """
 
     @functools.wraps(func)
     def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> None:
@@ -55,7 +62,11 @@ def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> None:
 def suggest_defining_url_if_401(
     func: Callable[_P, operations.PartitionResponse]
 ) -> Callable[_P, operations.PartitionResponse]:
-
+    """A decorator to suggest defining the 'server_url' parameter if a 401 Unauthorized error is encountered.
+    
+    This decorator addresses the common problem of users not passing in the 'server_url' when using their paid api key.
+    The decorator should be manually applied to General.partition after merging a PR from Speakeasy.
+    """
     @functools.wraps(func)
     def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> operations.PartitionResponse:
         try: