Skip to content

Commit 2042b2b

Browse files
Abel Milashclaude
andcommitted
Address PR review comments: docstrings, dynamic cap message, dispatch validation
- Fix .. note:: in records.create/update/upsert to mention concurrent dispatch when max_workers > 1, not just sequential chunking (comment #1) - Import _MAX_WORKERS in records.py and build the ValueError message from it so the text stays accurate if the cap changes (comment #2) - Add explicit ValueError in _dispatch_chunks for non-int or < 1 max_workers, with three new tests covering zero, negative, and non-int inputs (comments #4/#5) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0ed2574 commit 2042b2b

3 files changed

Lines changed: 44 additions & 14 deletions

File tree

src/PowerPlatform/Dataverse/data/_odata.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ def _dispatch_chunks(fn: Callable, chunks: List, max_workers: int) -> List:
8888
:param max_workers: Maximum number of concurrent worker threads.
8989
:return: List of results in chunk submission order.
9090
"""
91+
if not isinstance(max_workers, int) or max_workers < 1:
92+
raise ValueError(f"max_workers must be a positive integer; got {max_workers!r}")
9193
if max_workers > _MAX_WORKERS:
9294
warnings.warn(
9395
f"max_workers={max_workers} exceeds the maximum of {_MAX_WORKERS}; capping to {_MAX_WORKERS}.",

src/PowerPlatform/Dataverse/operations/records.py

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
from ..models.record import Record
1111
from ..models.upsert import UpsertItem
12+
from ..data._odata import _MAX_WORKERS
1213

1314
if TYPE_CHECKING:
1415
from ..client import DataverseClient
@@ -80,10 +81,12 @@ def create(
8081
:raises TypeError: If ``data`` is not a dict or list[dict].
8182
8283
.. note::
83-
Lists exceeding 1,000 records are automatically split into
84-
sequential chunks. This is **not atomic** — a failure mid-way
85-
may leave earlier chunks applied. Callers that require atomicity
86-
should limit input to ≤ 1,000 records.
84+
Lists exceeding 1,000 records are automatically split into chunks
85+
of up to 1,000 records; dispatched sequentially by default, or
86+
concurrently when ``max_workers > 1`` (capped to ``_MAX_WORKERS``).
87+
This is **not atomic** — a failure mid-way may leave earlier chunks
88+
applied. Callers that require atomicity should limit input to
89+
≤ 1,000 records.
8790
8891
Example:
8992
Create a single record::
@@ -100,7 +103,9 @@ def create(
100103
print(f"Created {len(guids)} accounts")
101104
"""
102105
if not isinstance(max_workers, int) or max_workers < 1:
103-
raise ValueError("max_workers must be a positive integer (1–3; values above 3 are capped at 3)")
106+
raise ValueError(
107+
f"max_workers must be a positive integer (1–{_MAX_WORKERS}; values above {_MAX_WORKERS} are capped at {_MAX_WORKERS})"
108+
)
104109
with self._client._scoped_odata() as od:
105110
entity_set = od._entity_set_from_schema_name(table)
106111
if isinstance(data, dict):
@@ -148,9 +153,11 @@ def update(
148153
does not match the expected pattern.
149154
150155
.. note::
151-
Lists exceeding 1,000 IDs are automatically split into sequential
152-
chunks. This is **not atomic** — a failure mid-way may leave
153-
earlier chunks applied. Callers that require atomicity should
156+
Lists exceeding 1,000 IDs are automatically split into chunks of
157+
up to 1,000; dispatched sequentially by default, or concurrently
158+
when ``max_workers > 1`` (capped to ``_MAX_WORKERS``). This is
159+
**not atomic** — a failure mid-way may leave earlier chunks
160+
applied. Callers that require atomicity should
154161
limit input to ≤ 1,000 IDs.
155162
156163
Example:
@@ -171,7 +178,9 @@ def update(
171178
)
172179
"""
173180
if not isinstance(max_workers, int) or max_workers < 1:
174-
raise ValueError("max_workers must be a positive integer (1–3; values above 3 are capped at 3)")
181+
raise ValueError(
182+
f"max_workers must be a positive integer (1–{_MAX_WORKERS}; values above {_MAX_WORKERS} are capped at {_MAX_WORKERS})"
183+
)
175184
with self._client._scoped_odata() as od:
176185
if isinstance(ids, str):
177186
if not isinstance(changes, dict):
@@ -507,10 +516,12 @@ def upsert(self, table: str, items: List[Union[UpsertItem, Dict[str, Any]]], *,
507516
dict with ``"alternate_key"`` and ``"record"`` keys.
508517
509518
.. note::
510-
Lists exceeding 1,000 items are automatically split into
511-
sequential chunks. This is **not atomic** — a failure mid-way
512-
may leave earlier chunks applied. Callers that require atomicity
513-
should limit input to ≤ 1,000 items.
519+
Lists exceeding 1,000 items are automatically split into chunks
520+
of up to 1,000; dispatched sequentially by default, or
521+
concurrently when ``max_workers > 1`` (capped to ``_MAX_WORKERS``).
522+
This is **not atomic** — a failure mid-way may leave earlier chunks
523+
applied. Callers that require atomicity should limit input to
524+
≤ 1,000 items.
514525
515526
Example:
516527
Upsert a single record using ``UpsertItem``::
@@ -566,7 +577,9 @@ def upsert(self, table: str, items: List[Union[UpsertItem, Dict[str, Any]]], *,
566577
``{"accountnumber": "ACC-001", "address1_postalcode": "98052"}``.
567578
"""
568579
if not isinstance(max_workers, int) or max_workers < 1:
569-
raise ValueError("max_workers must be a positive integer (1–3; values above 3 are capped at 3)")
580+
raise ValueError(
581+
f"max_workers must be a positive integer (1–{_MAX_WORKERS}; values above {_MAX_WORKERS} are capped at {_MAX_WORKERS})"
582+
)
570583
if not isinstance(items, list) or not items:
571584
raise TypeError("items must be a non-empty list of UpsertItem or dicts")
572585
normalized: List[UpsertItem] = []

tests/unit/data/test_multiple_chunking.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,6 +1141,21 @@ def test_below_cap_is_not_capped(self):
11411141
results = self._dispatch(lambda c: c, ["x", "y", "z"], max_workers=self._cap - 1)
11421142
self.assertEqual(results, ["x", "y", "z"])
11431143

1144+
def test_zero_raises_value_error(self):
1145+
"""max_workers=0 raises ValueError."""
1146+
with self.assertRaises(ValueError):
1147+
self._dispatch(lambda c: c, ["a"], max_workers=0)
1148+
1149+
def test_negative_raises_value_error(self):
1150+
"""Negative max_workers raises ValueError."""
1151+
with self.assertRaises(ValueError):
1152+
self._dispatch(lambda c: c, ["a"], max_workers=-1)
1153+
1154+
def test_non_int_raises_value_error(self):
1155+
"""Non-integer max_workers raises ValueError."""
1156+
with self.assertRaises(ValueError):
1157+
self._dispatch(lambda c: c, ["a"], max_workers="3")
1158+
11441159

11451160
# ---------------------------------------------------------------------------
11461161
# Picklist cache lock: concurrent cold-start

0 commit comments

Comments
 (0)