Skip to content

Commit 57416a7

Browse files
Add python-dotenv dependency and refine retry/pending logic
- Add python-dotenv to both PDM and uv dev dependencies so tests/client_test.py can run standalone without ModuleNotFoundError - Fix pre-commit default Python version from python3.9 to python3 - Refine structure_file pending detection: only set pending=True on 2xx responses (POST 422 indicates setup errors, not queued work) - Fix trailing newlines in utils.py and sample.env - Update tests to match new 422 pending behavior and fix docstring formatting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e61dc7a commit 57416a7

File tree

7 files changed

+65
-23
lines changed

7 files changed

+65
-23
lines changed

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# - Added pkgs feature flag auto generated code to flake8 exclude list
55
# Force all unspecified python hooks to run python 3.10
66
default_language_version:
7-
python: python3.9
7+
python: python3
88
default_stages:
99
- commit
1010
repos:

pyproject.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ test = [
3131
"pytest-dotenv>=0.5.2",
3232
"pytest-cov>=5.0.0",
3333
"pytest-md-report>=0.6.2",
34+
"python-dotenv>=1.0.0",
3435
]
3536
lint = [
3637
"autopep8~=2.0.2",
@@ -78,3 +79,8 @@ test.help = "Runs pytests for Unstract client"
7879
[build-system]
7980
requires = ["pdm-backend"]
8081
build-backend = "pdm.backend"
82+
83+
[dependency-groups]
84+
dev = [
85+
"python-dotenv>=1.0.0",
86+
]

src/unstract/api_deployments/client.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -357,15 +357,19 @@ def structure_file(self, file_paths: list[str]) -> dict:
357357
}
358358

359359
# Check if the status is pending or if it's successful but lacks a result.
360-
# Per the Unstract Status API migration guide (Option 1), we determine
361-
# pending state from the response body alone, ignoring the HTTP status
362-
# code — the server currently returns 422 for PENDING/EXECUTING.
363-
if execution_status in self.in_progress_statuses or (
364-
execution_status == "SUCCESS" and not extraction_result
365-
):
366-
obj_to_return.update(
367-
{"status_check_api_endpoint": status_api_endpoint, "pending": True}
368-
)
360+
# The POST endpoint returns 200 for successful queuing (including
361+
# PENDING/EXECUTING) and 422 only on setup errors — guard against
362+
# incorrectly polling after an error response.
363+
if 200 <= response.status_code < 300:
364+
if execution_status in self.in_progress_statuses or (
365+
execution_status == "SUCCESS" and not extraction_result
366+
):
367+
obj_to_return.update(
368+
{
369+
"status_check_api_endpoint": status_api_endpoint,
370+
"pending": True,
371+
}
372+
)
369373

370374
return obj_to_return
371375

src/unstract/api_deployments/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,4 @@ def redact_key(api_key: str, reveal_length=4) -> str:
1919
redacted_length = max(len(api_key) - reveal_length, 0)
2020
revealed_part = api_key[:reveal_length]
2121
redacted_part = "x" * redacted_length
22-
return revealed_part + redacted_part
22+
return revealed_part + redacted_part

tests/sample.env

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
API_URL="http://localhost:8000/deployment/api/<org_id>/<api_name>/"
22
UNSTRACT_API_DEPLOYMENT_KEY="your-api-key-without-bearer-prefix"
3-
TEST_FILES="/path/to/test1.pdf,/path/to/test2.pdf"
3+
TEST_FILES="/path/to/test1.pdf,/path/to/test2.pdf"

tests/test_retry.py

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,9 @@ class TestTimeoutPassed:
331331
@patch("unstract.api_deployments.client.requests.request")
332332
def test_no_default_timeout_set(self, mock_request, client):
333333
"""api_timeout is a server-side parameter, not an HTTP socket timeout.
334-
_request_with_retry should NOT inject a default timeout kwarg."""
334+
335+
_request_with_retry should NOT inject a default timeout kwarg.
336+
"""
335337
mock_request.return_value = _mock_response(200)
336338
client._request_with_retry("GET", "https://api.example.com/test")
337339
_, kwargs = mock_request.call_args
@@ -476,7 +478,8 @@ def test_422_with_executing_status_sets_pending_true(self, mock_request, client)
476478

477479
@patch("unstract.api_deployments.client.requests.request")
478480
def test_422_with_pending_status_sets_pending_true(self, mock_request, client):
479-
"""HTTP 422 with PENDING body status — still detected via body check."""
481+
"""HTTP 422 with PENDING body status — still detected via body
482+
check."""
480483
mock_request.return_value = _mock_response(
481484
422, json_data={"status": "PENDING", "error": "", "message": ""}
482485
)
@@ -591,12 +594,20 @@ def test_structure_file_sync_mode_default_timeout(self, mock_post, tmp_path):
591594
assert mock_post.call_count == 1
592595

593596

594-
# ── structure_file: 422 + PENDING sets pending=True ──
597+
# ── structure_file: POST 422 does NOT set pending ──
598+
# The POST endpoint returns 200 for successful queuing (PENDING/EXECUTING)
599+
# and 422 only on setup errors. A 422 should never trigger polling.
595600

596601

597-
class TestStructureFilePendingOn422:
602+
class TestStructureFile422DoesNotSetPending:
598603
@patch("unstract.api_deployments.client.requests.post")
599-
def test_422_pending_sets_pending_true(self, mock_post, tmp_path):
604+
def test_422_pending_does_not_set_pending(self, mock_post, tmp_path):
605+
"""POST 422 with PENDING status should NOT set pending=True.
606+
607+
The POST endpoint returns 422 only on setup errors, so the
608+
client must not start polling even if execution_status says
609+
PENDING.
610+
"""
600611
test_file = tmp_path / "test.pdf"
601612
test_file.write_bytes(b"fake pdf content")
602613

@@ -618,12 +629,16 @@ def test_422_pending_sets_pending_true(self, mock_post, tmp_path):
618629
},
619630
)
620631
result = c.structure_file([str(test_file)])
621-
assert result["pending"] is True
622-
assert result["status_check_api_endpoint"] == "/api/status/abc"
632+
assert result["pending"] is False
623633
assert result["status_code"] == 422
624634

625635
@patch("unstract.api_deployments.client.requests.post")
626-
def test_422_executing_sets_pending_true(self, mock_post, tmp_path):
636+
def test_422_executing_does_not_set_pending(self, mock_post, tmp_path):
637+
"""POST 422 with EXECUTING status should NOT set pending=True.
638+
639+
Same rationale as above — 422 from POST means a setup error, not
640+
a legitimately queued execution.
641+
"""
627642
test_file = tmp_path / "test.pdf"
628643
test_file.write_bytes(b"fake pdf content")
629644

@@ -645,12 +660,12 @@ def test_422_executing_sets_pending_true(self, mock_post, tmp_path):
645660
},
646661
)
647662
result = c.structure_file([str(test_file)])
648-
assert result["pending"] is True
663+
assert result["pending"] is False
649664
assert result["status_code"] == 422
650665

651666
@patch("unstract.api_deployments.client.requests.post")
652-
def test_200_pending_still_works(self, mock_post, tmp_path):
653-
"""Existing behaviour: 200 + PENDING also sets pending=True."""
667+
def test_200_pending_sets_pending_true(self, mock_post, tmp_path):
668+
"""POST 200 + PENDING correctly sets pending=True for polling."""
654669
test_file = tmp_path / "test.pdf"
655670
test_file.write_bytes(b"fake pdf content")
656671

uv.lock

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)