Skip to content

Commit 53cd698

Browse files
authored
enhancing python SDK test coverage (#47360)
* fix adding location_cache and decoding integration/unit tests * fix adding more location_cache tests and cleaning up comments * fix adding CosmosItemPaged tests and cleaning up comments * fix adding partition splits pagination tests and cleaning up comments * fix adding transiently inconsistent snapshot handling tests and cleaning up comments * fix adding timeout related tests and cleaning up comments * fix adding response hook and availability strategy tests and cleaning up comments * fix adding regional endpoints and compund session token tests and cleaning up comments * fix adding user-agent and read-timeout tests and cleaning up comments * fix - cleaning up comments * fix - adding bounded memory tests * fix - fixing user agent and memory tests * fix - addressing co-pilot comments * fix - fixing failing tests * fix - adding more tests * cleaning up resources * addressing PR comments * Add generated api.md and api.metadata.yml for azure-cosmos * addressing spell check warnings
1 parent bb4d149 commit 53cd698

38 files changed

Lines changed: 8252 additions & 734 deletions
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
apiMdSha256: de3b8c0f2a0405fac7152d562c982bf98bf7bb546c46f05e39164574e47f7f7a
22
parserVersion: 0.3.28
3-
pythonVersion: 3.13.14
3+
pythonVersion: 3.13.14

sdk/cosmos/azure-cosmos/azure/cosmos/_location_cache.py

Lines changed: 15 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,14 @@
3939

4040

4141
def _clean_location_list(locations: Optional[Sequence[Optional[str]]]) -> list[str]:
42-
"""Drop ``None``/empty/whitespace-only entries from a location list.
42+
"""Return the list with None, empty, and whitespace-only entries removed.
4343
44-
We filter these out at every intake point because the region normalizer
45-
turns ``None`` (and empty/whitespace input) into an empty string. If such
46-
a value were allowed into an exclusion list, every endpoint that wasn't
47-
found in our by-endpoint lookup map — which also returns an empty string
48-
as its default — would compare equal to it and get silently excluded.
49-
Stripping the bad inputs once, at the boundary, prevents that collision.
44+
Used at every location-list entry point so blank inputs cannot accidentally
45+
match real endpoints during later comparisons.
5046
51-
:param locations: The raw location list to clean, or ``None``. May contain
52-
``None`` entries and empty/whitespace-only strings.
47+
:param locations: The raw list of region names, or None.
5348
:type locations: Optional[Sequence[Optional[str]]]
54-
:return: A new list containing only the non-empty, non-whitespace string
55-
entries from ``locations``. Returns an empty list when ``locations``
56-
is ``None`` or empty.
49+
:return: The cleaned list. Empty when input is None or empty.
5750
:rtype: list[str]
5851
"""
5952
if not locations:
@@ -62,41 +55,15 @@ def _clean_location_list(locations: Optional[Sequence[Optional[str]]]) -> list[s
6255

6356

6457
def _normalize_region_name(region_name: Optional[str]) -> str:
65-
"""Canonicalize a region name for equality comparison.
66-
67-
This is the single function every region-name comparison in this module
68-
routes through (exclusion matching, preferred-location resolution, the
69-
by-endpoint normalized lookup, the config-mismatch warning emitter, and
70-
the most-preferred check in ``should_refresh_endpoints``). Two names
71-
normalize to the same string iff the SDK treats them as the same region.
72-
73-
The rule, in plain terms:
74-
75-
- **Stripped:** outer and inner whitespace (via ``.strip()`` followed by
76-
``.split()`` / ``"".join(...)``), case (``.lower()``), and the two
77-
separator characters customers commonly write — ``-`` and ``_``.
78-
- **Preserved:** ASCII letters and **digits**. The digit invariant is the
79-
load-bearing one: it is what keeps ``"East US"`` and ``"East US 2"``
80-
distinct after normalization (``"eastus"`` vs ``"eastus2"``). Stripping
81-
digits, even as part of a well-meaning cleanup, would silently collide
82-
these regions and route each one's traffic to the other.
83-
- **``None`` / empty / whitespace-only input maps to ``""``.** That empty
84-
string is a sentinel that also appears as the default value of the
85-
by-endpoint name lookup, so callers must filter such inputs out at the
86-
configuration boundary with :func:`_clean_location_list` before they
87-
reach this function. See that function's docstring for the collision
88-
scenario.
89-
- **Idempotent.** ``_normalize_region_name(_normalize_region_name(x)) ==
90-
_normalize_region_name(x)`` for every ``x``. Defensive double-calls are
91-
safe.
92-
93-
:param region_name: A region name to canonicalize, or ``None``. Customer
94-
input (preferred/excluded locations) and account-side names from the
95-
gateway both flow through here.
58+
"""Return a canonical form of a region name for equality checks.
59+
60+
Lowercases the text and strips spaces, hyphens, and underscores so values
61+
like "East US 2", "east-us-2", and "east_us_2" compare equal. Digits are
62+
kept so "East US" and "East US 2" stay distinct. None becomes "".
63+
64+
:param region_name: A region name, or None.
9665
:type region_name: Optional[str]
97-
:return: The canonicalized form, suitable for direct ``==`` / ``in``
98-
comparison against other canonicalized names. Returns ``""`` for
99-
``None`` or whitespace-only input.
66+
:return: The canonical form, or "" for None or whitespace-only input.
10067
:rtype: str
10168
"""
10269
if region_name is None:
@@ -648,9 +615,8 @@ def update_location_cache(self, write_locations=None, read_locations=None, enabl
648615
self._read_locations_by_normalized,
649616
)
650617

651-
# Config-time visibility for misconfigured region names. Dedupe ensures periodic
652-
# refreshes do not re-emit identical warnings; new mismatches still surface because
653-
# the dedupe key includes the available account regions snapshot.
618+
# Warn once when configured region names do not match any account region.
619+
# Repeated identical warnings are suppressed; a different set of regions emits a new one.
654620
if self.connection_policy.PreferredLocations:
655621
self._emit_config_mismatch_warning_once(
656622
self.connection_policy.PreferredLocations,

sdk/cosmos/azure-cosmos/tests/routing/test_routing_map_provider.py

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,131 @@ def refresher_fn():
921921
self.assertEqual(none_seen['count'], 0,
922922
"Cache entry should never be None during a refresh — it should be atomically replaced")
923923

924+
# The tests below run through SmartRoutingMapProvider to confirm that a
925+
# bad cache snapshot surfaces as a CosmosHttpResponseError the caller can
926+
# handle, not as a raw ValueError or AssertionError.
927+
928+
class _SequencedSnapshotClient(object):
929+
"""Mock client that returns the next payload from response_sequence on
930+
each fresh read, and an empty page when the If-None-Match matches the
931+
last etag (acts like a 304 reply)."""
932+
933+
def __init__(self, response_sequence):
934+
self.response_sequence = response_sequence
935+
self.url_connection = "https://mock-sequenced-test.documents.azure.com:443/"
936+
self.call_count = 0
937+
self._last_etag = None
938+
939+
def _ReadPartitionKeyRanges(self, _collection_link, _feed_options=None, **kwargs):
940+
headers_in = kwargs.get('headers') or {}
941+
inm = headers_in.get('If-None-Match')
942+
if inm is not None and inm == self._last_etag:
943+
status_capture = kwargs.get('_internal_response_status_capture')
944+
if status_capture is not None:
945+
status_capture[0] = 304
946+
captured_headers = kwargs.get('_internal_response_headers_capture')
947+
if captured_headers is not None:
948+
captured_headers.clear()
949+
captured_headers.update({'ETag': self._last_etag})
950+
return []
951+
idx = min(self.call_count, len(self.response_sequence) - 1)
952+
payload = self.response_sequence[idx]
953+
self.call_count += 1
954+
etag = f'"etag-{self.call_count}"'
955+
self._last_etag = etag
956+
captured_headers = kwargs.get('_internal_response_headers_capture')
957+
if captured_headers is not None:
958+
captured_headers.clear()
959+
captured_headers.update({'ETag': etag})
960+
status_capture = kwargs.get('_internal_response_status_capture')
961+
if status_capture is not None:
962+
status_capture[0] = 200
963+
return payload
964+
965+
_OVERLAP_PAYLOAD = [
966+
{'id': 'L', 'minInclusive': '', 'maxExclusive': '80'},
967+
{'id': '10', 'minInclusive': '80', 'maxExclusive': 'A0'},
968+
{'id': '10/0', 'minInclusive': '80', 'maxExclusive': '90'},
969+
{'id': '10/1', 'minInclusive': '90', 'maxExclusive': 'A0'},
970+
{'id': 'R', 'minInclusive': 'A0', 'maxExclusive': 'FF'},
971+
]
972+
_GAP_PAYLOAD = [
973+
{'id': 'L', 'minInclusive': '', 'maxExclusive': '80'},
974+
{'id': 'R', 'minInclusive': 'A0', 'maxExclusive': 'FF'},
975+
]
976+
_GOOD_PAYLOAD = [
977+
{'id': 'L', 'minInclusive': '', 'maxExclusive': '80'},
978+
{'id': '10/0', 'minInclusive': '80', 'maxExclusive': '90', 'parents': ['10']},
979+
{'id': '10/1', 'minInclusive': '90', 'maxExclusive': 'A0', 'parents': ['10']},
980+
{'id': 'R', 'minInclusive': 'A0', 'maxExclusive': 'FF'},
981+
]
982+
983+
def _reset_shared_cache_state(self, provider):
984+
"""Release the given provider and clear shared cache dicts so the next
985+
sub-test or run starts with a clean slate."""
986+
provider.release()
987+
with _shared_cache_lock:
988+
_shared_routing_map_cache.clear()
989+
_shared_collection_locks.clear()
990+
_shared_locks_locks.clear()
991+
_shared_cache_refcounts.clear()
992+
993+
def test_smart_provider_does_not_leak_overlap_value_error_on_persistent_inconsistency(self):
994+
"""A persistent overlap or gap snapshot must raise 503 with sub_status
995+
21015 from SmartRoutingMapProvider.get_overlapping_ranges, not a bare
996+
ValueError or AssertionError."""
997+
full_range = routing_range.Range("", "FF", True, False)
998+
999+
for label, payload in (("overlap", self._OVERLAP_PAYLOAD), ("gap", self._GAP_PAYLOAD)):
1000+
with self.subTest(snapshot=label):
1001+
client = TestRoutingMapProvider._SequencedSnapshotClient([payload])
1002+
provider = SmartRoutingMapProvider(client)
1003+
try:
1004+
with patch(
1005+
'azure.cosmos._routing.routing_map_provider.time.sleep',
1006+
return_value=None,
1007+
):
1008+
with self.assertRaises(CosmosHttpResponseError) as ctx:
1009+
provider.get_overlapping_ranges(
1010+
"dbs/db/colls/container", [full_range]
1011+
)
1012+
exc = ctx.exception
1013+
self.assertEqual(
1014+
exc.status_code,
1015+
http_constants.StatusCodes.SERVICE_UNAVAILABLE,
1016+
f"Persistent {label} snapshot must surface as 503.",
1017+
)
1018+
self.assertEqual(
1019+
exc.sub_status,
1020+
http_constants.SubStatusCodes.ROUTING_MAP_SNAPSHOT_INCONSISTENT,
1021+
f"503 from a persistent {label} must set sub_status to 21015.",
1022+
)
1023+
self.assertNotIsInstance(exc, AssertionError)
1024+
self.assertFalse(isinstance(exc, ValueError))
1025+
finally:
1026+
self._reset_shared_cache_state(provider)
1027+
1028+
def test_smart_provider_recovers_through_full_stack_after_transient_overlap(self):
1029+
"""A bad overlap response followed by a good one must return the
1030+
expected ranges from get_overlapping_ranges."""
1031+
full_range = routing_range.Range("", "FF", True, False)
1032+
client = TestRoutingMapProvider._SequencedSnapshotClient(
1033+
[self._OVERLAP_PAYLOAD, self._GOOD_PAYLOAD]
1034+
)
1035+
provider = SmartRoutingMapProvider(client)
1036+
try:
1037+
with patch(
1038+
'azure.cosmos._routing.routing_map_provider.time.sleep',
1039+
return_value=None,
1040+
):
1041+
overlapping = provider.get_overlapping_ranges(
1042+
"dbs/db/colls/container", [full_range]
1043+
)
1044+
ids = [r['id'] for r in overlapping]
1045+
self.assertEqual(ids, ['L', '10/0', '10/1', 'R'])
1046+
finally:
1047+
self._reset_shared_cache_state(provider)
1048+
9241049
if __name__ == "__main__":
9251050
# import sys;sys.argv = ['', 'Test.testName']
9261051
unittest.main()

0 commit comments

Comments
 (0)