Skip to content

Commit f2554a8

Browse files
committed
PR feedback: ValueError to warning, better method name, fix type
1 parent 236d249 commit f2554a8

2 files changed

Lines changed: 24 additions & 13 deletions

File tree

python/lib/sift_client/_tests/resources/test_rules.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ class TestBatchUpdate:
620620
"""Tests for the async batch_update_rules method."""
621621

622622
@pytest.mark.asyncio
623-
async def test_batch_update_rules(self, rules_api_async, nostromo_asset):
623+
async def test_batch_update_or_create_rules(self, rules_api_async, nostromo_asset):
624624
"""Test updating multiple rules with different fields."""
625625
from datetime import datetime, timezone
626626

@@ -672,7 +672,7 @@ async def test_batch_update_rules(self, rules_api_async, nostromo_asset):
672672

673673
updates = [rule1_update, rule2_update]
674674

675-
updated_rules = await rules_api_async.batch_update_rules(updates)
675+
updated_rules = await rules_api_async.batch_update_or_create_rules(updates)
676676

677677
assert isinstance(updated_rules, list)
678678
assert len(updated_rules) == 2
@@ -727,7 +727,7 @@ async def test_batch_update_rules_creates_rules(self, rules_api_async, nostromo_
727727
try:
728728
# Batch update (actually create) both rules
729729
updates = [rule1, rule2]
730-
updated_rules = await rules_api_async.batch_update_rules(updates)
730+
updated_rules = await rules_api_async.batch_update_or_create_rules(updates)
731731

732732
assert isinstance(updated_rules, list)
733733
assert len(updated_rules) == 2
@@ -741,7 +741,7 @@ async def test_batch_update_rules_creates_rules(self, rules_api_async, nostromo_
741741
@pytest.mark.asyncio
742742
async def test_batch_update_rules_empty_list(self, rules_api_async):
743743
"""Test handling empty list."""
744-
updated_rules = await rules_api_async.batch_update_rules([])
744+
updated_rules = await rules_api_async.batch_update_or_create_rules([])
745745

746746
assert isinstance(updated_rules, list)
747747
assert len(updated_rules) == 0

python/lib/sift_client/resources/rules.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
from __future__ import annotations
22

3+
import warnings
34
from typing import TYPE_CHECKING, Any, Sequence
45

56
from sift_client._internal.low_level_wrappers.rules import RulesLowLevelClient
7+
from sift_client.errors import SiftWarning
68
from sift_client.resources._base import ResourceBase
79
from sift_client.sift_types.asset import Asset
810
from sift_client.sift_types.rule import Rule, RuleCreate, RuleUpdate
@@ -177,10 +179,13 @@ async def create(
177179
create: A RuleCreate object, a dictionary with configuration for the new rule, or a list of the previously mentioned objects.
178180
override_expression_validation: When true, the rule will be created even if the expression is invalid.
179181
182+
Warnings:
183+
SiftWarning: If not all rules are created.
184+
180185
Returns:
181186
The created Rule (if a single dictionary or RuleCreate was provided) otherwise a list of the created rules.
182187
"""
183-
rules: list[RuleCreate | RuleUpdate] = []
188+
rules: list[RuleCreate] = []
184189
if isinstance(create, Sequence):
185190
for c in create:
186191
if isinstance(c, dict):
@@ -192,16 +197,18 @@ async def create(
192197
else:
193198
rules.append(create)
194199

195-
created_rules = await self.batch_update_rules(
200+
created_rules = await self.batch_update_or_create_rules(
196201
rules=rules, override_expression_validation=override_expression_validation
197202
)
198203
if len(created_rules) != len(rules):
199-
raise ValueError(
200-
f"Failed to create all rules: got {len(created_rules)} but expected {len(rules)}"
204+
warnings.warn(
205+
f"Failed to create all rules: got {len(created_rules)} but expected {len(rules)}",
206+
SiftWarning,
207+
stacklevel=2,
201208
)
202209

203210
# If there is only one rule to create provided as a dict or RuleCreate, return the single rule.
204-
if len(created_rules) == 1 and not isinstance(create, list):
211+
if len(created_rules) == 1 and not isinstance(create, Sequence):
205212
return created_rules[0]
206213

207214
# Otherwise, return the list of created rules.
@@ -260,7 +267,7 @@ async def unarchive(self, rule: str | Rule) -> Rule:
260267
"""
261268
return await self.update(rule=rule, update=RuleUpdate(is_archived=False))
262269

263-
async def batch_update_rules(
270+
async def batch_update_or_create_rules(
264271
self,
265272
rules: Sequence[RuleCreate | RuleUpdate],
266273
*,
@@ -272,6 +279,9 @@ async def batch_update_rules(
272279
rules: List of rule creates or updates to apply. RuleUpdate objects must have resource_id set.
273280
override_expression_validation: When true, the rules will be created even if the expressions are invalid.
274281
282+
Warnings:
283+
UserWarning: If not all rules are created or updated.
284+
275285
Returns:
276286
List of updated or created Rules.
277287
@@ -300,9 +310,10 @@ async def batch_update_rules(
300310

301311
# Ensure all rules were updated/created.
302312
if response.rules_created_count + response.rules_updated_count != len(rules):
303-
raise ValueError(
304-
f"Not all rules were updated/created: got {response.rules_created_count + response.rules_updated_count} "
305-
f"but expected {len(rules)}"
313+
warnings.warn(
314+
f"Not all rules were updated/created: got {response.rules_created_count + response.rules_updated_count} but expected {len(rules)}",
315+
SiftWarning,
316+
stacklevel=2,
306317
)
307318

308319
# Collect rule IDs from the response

0 commit comments

Comments
 (0)