Skip to content

Commit b353b15

Browse files
authored
[DPE-1657] Address launch_synstage error in demo.py (#53) (#54)
* robust error-handling for unexpected API changes * pass flake8 tests * serialize tower integration tests * new client mock tests. new CONTRIBUTING.md section. removed xdist_grouping * fix line too long
1 parent f901bbf commit b353b15

3 files changed

Lines changed: 76 additions & 5 deletions

File tree

CONTRIBUTING.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,3 +302,17 @@ on [PyPI], the following steps can be used to release a new version for
302302
[virtualenv]: https://virtualenv.pypa.io/en/stable/
303303
[repository]: https://github.com/sage-bionetworks-workflows/py-orca
304304
[issue tracker]: https://github.com/sage-bionetworks-workflows/py-orca/issues
305+
306+
---
307+
308+
## Troubleshooting CI
309+
310+
> This is a living document. As new CI failures are diagnosed and resolved, add them here so future contributors don't have to rediscover the same issues.
311+
312+
### Integration tests fail with `401 Unauthorized` or unexpected API errors
313+
314+
**Symptom:** Integration tests (e.g., `test_that_a_workflow_can_be_launched`) fail in CI with authentication or API errors, even though the tests pass locally.
315+
316+
**Cause:** The `NEXTFLOWTOWER_CONNECTION_URI` GitHub Actions secret MAY contain an expired or outdated API token.
317+
318+
**Fix:** Regenerate a valid Nextflow Tower API token following the format outlined in `.env.example` and update the secret in the repository's GitHub Actions settings (`Settings > Secrets and variables > Actions > NEXTFLOWTOWER_CONNECTION_URI`).

src/orca/services/nextflowtower/client.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import warnings
12
from typing import Any, Optional
23

34
import requests
@@ -91,16 +92,24 @@ def request_json(self, method: str, path: str, **kwargs) -> dict[str, Any]:
9192
return response.json()
9293

9394
def request_paged(self, method: str, path: str, **kwargs) -> dict[str, Any]:
94-
"""Iterate through pages of results for a given request.
95+
"""Paginate through all pages of a paged API endpoint and collect results.
96+
97+
Sends repeated requests, incrementing the offset each time, until all
98+
items have been retrieved. Expects each response to contain a size key
99+
(``totalSize`` or ``total``) and exactly one list-valued key holding
100+
the items for that page. Warns if multiple list-valued keys are found
101+
and uses the first. Raises if no list-valued key is found.
95102
96103
See ``TowerClient.request`` for argument definitions.
97104
98105
Raises:
99-
HTTPError: If the response doesn't match the expectation
100-
for a paged endpoint.
106+
HTTPError: If the response contains no list-valued key, or if the
107+
total number of collected items does not match the declared
108+
total size.
101109
102110
Returns:
103-
The cumulative list of items from all pages.
111+
A dict with ``totalSize`` (or ``total``) and the items key mapped
112+
to the full combined list across all pages.
104113
"""
105114
# Ensure defaults for pagination query parameters
106115
self.update_kwarg(kwargs, "params", "max", 50)
@@ -114,7 +123,21 @@ def request_paged(self, method: str, path: str, **kwargs) -> dict[str, Any]:
114123
kwargs["params"]["offset"] = num_items
115124
json = self.request_json(method, path, **kwargs)
116125
total_size = json.pop("totalSize", None) or json.pop("total", 0)
117-
key_name, items = json.popitem()
126+
list_keys = [(k, v) for k, v in json.items() if isinstance(v, list)]
127+
if not list_keys:
128+
received = {k: type(v).__name__ for k, v in json.items()}
129+
raise HTTPError(
130+
f"Paged response contained no list-valued key. "
131+
f"Received keys/types: {received}"
132+
)
133+
if len(list_keys) > 1:
134+
warnings.warn(
135+
f"Paged response contained multiple list-valued keys: "
136+
f"{[k for k, _ in list_keys]}. "
137+
f"Using the first: '{list_keys[0][0]}'.",
138+
stacklevel=2,
139+
)
140+
key_name, items = list_keys[0]
118141
num_items += len(items)
119142
all_items.extend(items)
120143

tests/services/nextflowtower/test_client.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import warnings
2+
13
import pytest
24
from requests.exceptions import HTTPError
35

@@ -166,3 +168,35 @@ def test_that_get_task_logs_works(client, mocker, get_response):
166168
)
167169
mock.assert_called()
168170
assert result == "Ciao world!"
171+
172+
173+
def test_that_request_paged_raises_when_no_list_key(client, mocker):
174+
mock = mocker.patch.object(client, "request_json")
175+
mock.return_value = {"totalSize": 1, "hasMore": True}
176+
with pytest.raises(HTTPError, match="no list-valued key"):
177+
client.list_labels(98765)
178+
179+
180+
def test_that_request_paged_warns_when_multiple_list_keys(client, mocker):
181+
mock = mocker.patch.object(client, "request_json")
182+
mock.return_value = {
183+
"totalSize": 1,
184+
"labels": [{"id": 1, "name": "foo", "value": None, "resource": False}],
185+
"extras": [{"id": 2}],
186+
}
187+
with warnings.catch_warnings(record=True) as caught:
188+
warnings.simplefilter("always")
189+
client.list_labels(98765)
190+
assert len(caught) == 1
191+
assert "multiple list-valued keys" in str(caught[0].message)
192+
193+
194+
def test_that_request_paged_tolerates_extra_boolean_fields(client, mocker):
195+
mock = mocker.patch.object(client, "request_json")
196+
mock.return_value = {
197+
"totalSize": 1,
198+
"labels": [{"id": 1, "name": "foo", "value": None, "resource": False}],
199+
"hasMoreEntries": True,
200+
}
201+
result = client.list_labels(98765)
202+
assert len(result) == 1

0 commit comments

Comments
 (0)