Skip to content

Commit 6b82982

Browse files
committed
refactor: complete SDK architecture and quality improvements
Major improvements for PyPI readiness: Architecture & Standards: - Align with Microsoft Azure SDK patterns (explicit imports only) - Remove re-exports from all submodule __init__.py files - Fix py2docfx documentation duplication issues - Clean up verbose docstring additions for professional appearance Code Quality: - Fix deprecated datetime.utcnow() → datetime.now(timezone.utc) - Update explicit imports: error_codes module usage consistency - Eliminate all test warnings (pytest collection + deprecation) - Rename test helper classes: TestClient→MockClient, TestableClient→MockableClient Import Consistency: - Replace 'from core import error_codes as ec' with direct imports - Use explicit imports throughout codebase for better maintainability - All HTTP error constants and functions imported directly Result: Zero warnings, Microsoft-compliant architecture, PyPI-ready SDK All tests pass, no breaking changes, professional documentation structure
1 parent f0f7dd8 commit 6b82982

7 files changed

Lines changed: 39 additions & 44 deletions

File tree

src/PowerPlatform/Dataverse/core/__init__.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,6 @@
66
77
This module contains the foundational components including authentication,
88
configuration, HTTP client, and error handling.
9-
10-
Import classes directly from their specific modules:
11-
- Authentication: from .auth import AuthManager, TokenPair
12-
- Configuration: from .config import DataverseConfig
13-
- Errors: from .errors import DataverseError, HttpError, etc.
14-
- HTTP Client: from .http import HttpClient
159
"""
1610

17-
# No re-exports to avoid documentation duplication with py2docfx
1811
__all__ = []

src/PowerPlatform/Dataverse/core/errors.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def __init__(
5353
self.details = details or {}
5454
self.source = source or "client"
5555
self.is_transient = is_transient
56-
self.timestamp = _dt.datetime.utcnow().isoformat() + "Z"
56+
self.timestamp = _dt.datetime.now(_dt.timezone.utc).isoformat().replace('+00:00', 'Z')
5757

5858
def to_dict(self) -> Dict[str, Any]:
5959
"""

src/PowerPlatform/Dataverse/data/__init__.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,6 @@
66
77
This module contains OData protocol handling, CRUD operations, metadata management,
88
SQL query functionality, and file upload capabilities.
9-
10-
Import classes directly from their specific modules:
11-
- OData operations: from .odata import ODataClient
12-
- File uploads: from .upload import ODataFileUpload
139
"""
1410

15-
# No re-exports to avoid documentation duplication with py2docfx
1611
__all__ = []

src/PowerPlatform/Dataverse/data/odata.py

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,18 @@
1717
from ..core.http import HttpClient
1818
from .upload import ODataFileUpload
1919
from ..core.errors import *
20-
from ..core import error_codes as ec
20+
from ..core.error_codes import (
21+
http_subcode,
22+
is_transient_status,
23+
VALIDATION_SQL_NOT_STRING,
24+
VALIDATION_SQL_EMPTY,
25+
METADATA_ENTITYSET_NOT_FOUND,
26+
METADATA_ENTITYSET_NAME_MISSING,
27+
METADATA_TABLE_NOT_FOUND,
28+
METADATA_TABLE_ALREADY_EXISTS,
29+
METADATA_COLUMN_NOT_FOUND,
30+
VALIDATION_UNSUPPORTED_CACHE_KIND,
31+
)
2132

2233
from ..__version__ import __version__ as _SDK_VERSION
2334

@@ -121,7 +132,7 @@ def _request(self, method: str, url: str, *, expected: tuple[int, ...] = (200, 2
121132
except Exception:
122133
pass
123134
sc = r.status_code
124-
subcode = ec.http_subcode(sc)
135+
subcode = http_subcode(sc)
125136
correlation_id = headers.get("x-ms-correlation-request-id") or headers.get("x-ms-correlation-id")
126137
request_id = headers.get("x-ms-client-request-id") or headers.get("request-id") or headers.get("x-ms-request-id")
127138
traceparent = headers.get("traceparent")
@@ -132,7 +143,7 @@ def _request(self, method: str, url: str, *, expected: tuple[int, ...] = (200, 2
132143
retry_after = int(ra)
133144
except Exception:
134145
retry_after = None
135-
is_transient = ec.is_transient_status(sc)
146+
is_transient = is_transient_status(sc)
136147
raise HttpError(
137148
msg,
138149
status_code=sc,
@@ -558,9 +569,9 @@ def _query_sql(self, sql: str) -> list[dict[str, Any]]:
558569
Endpoint form: ``GET /{entity_set}?sql=<encoded select>``. The client extracts the logical table name, resolves the entity set (metadata cached), then issues the request. Only a constrained SELECT subset is supported by the platform.
559570
"""
560571
if not isinstance(sql, str):
561-
raise ValidationError("sql must be a string", subcode=ec.VALIDATION_SQL_NOT_STRING)
572+
raise ValidationError("sql must be a string", subcode=VALIDATION_SQL_NOT_STRING)
562573
if not sql.strip():
563-
raise ValidationError("sql must be a non-empty string", subcode=ec.VALIDATION_SQL_EMPTY)
574+
raise ValidationError("sql must be a non-empty string", subcode=VALIDATION_SQL_EMPTY)
564575
sql = sql.strip()
565576

566577
# Extract logical table name via helper (robust to identifiers ending with 'from')
@@ -631,14 +642,14 @@ def _entity_set_from_logical(self, logical: str) -> str:
631642
plural_hint = " (did you pass a plural entity set name instead of the singular logical name?)" if logical.endswith("s") and not logical.endswith("ss") else ""
632643
raise MetadataError(
633644
f"Unable to resolve entity set for logical name '{logical}'. Provide the singular logical name.{plural_hint}",
634-
subcode=ec.METADATA_ENTITYSET_NOT_FOUND,
645+
subcode=METADATA_ENTITYSET_NOT_FOUND,
635646
)
636647
md = items[0]
637648
es = md.get("EntitySetName")
638649
if not es:
639650
raise MetadataError(
640651
f"Metadata response missing EntitySetName for logical '{logical}'.",
641-
subcode=ec.METADATA_ENTITYSET_NAME_MISSING,
652+
subcode=METADATA_ENTITYSET_NAME_MISSING,
642653
)
643654
self._logical_to_entityset_cache[logical] = es
644655
primary_id_attr = md.get("PrimaryIdAttribute")
@@ -1150,7 +1161,7 @@ def _delete_table(self, tablename: str) -> None:
11501161
if not ent or not ent.get("MetadataId"):
11511162
raise MetadataError(
11521163
f"Table '{entity_schema}' not found.",
1153-
subcode=ec.METADATA_TABLE_NOT_FOUND,
1164+
subcode=METADATA_TABLE_NOT_FOUND,
11541165
)
11551166
metadata_id = ent["MetadataId"]
11561167
url = f"{self.api}/EntityDefinitions({metadata_id})"
@@ -1191,7 +1202,7 @@ def _create_table(
11911202
if ent:
11921203
raise MetadataError(
11931204
f"Table '{entity_schema}' already exists.",
1194-
subcode=ec.METADATA_TABLE_ALREADY_EXISTS,
1205+
subcode=METADATA_TABLE_ALREADY_EXISTS,
11951206
)
11961207

11971208
created_cols: List[str] = []
@@ -1254,7 +1265,7 @@ def _create_columns(
12541265
if not ent or not ent.get("MetadataId"):
12551266
raise MetadataError(
12561267
f"Table '{entity_schema}' not found.",
1257-
subcode=ec.METADATA_TABLE_NOT_FOUND,
1268+
subcode=METADATA_TABLE_NOT_FOUND,
12581269
)
12591270

12601271
metadata_id = ent.get("MetadataId")
@@ -1317,7 +1328,7 @@ def _delete_columns(
13171328
if not ent or not ent.get("MetadataId"):
13181329
raise MetadataError(
13191330
f"Table '{entity_schema}' not found.",
1320-
subcode=ec.METADATA_TABLE_NOT_FOUND,
1331+
subcode=METADATA_TABLE_NOT_FOUND,
13211332
)
13221333

13231334
metadata_id = ent.get("MetadataId")
@@ -1330,7 +1341,7 @@ def _delete_columns(
13301341
if not attr_meta:
13311342
raise MetadataError(
13321343
f"Column '{schema_name}' not found on table '{entity_schema}'.",
1333-
subcode=ec.METADATA_COLUMN_NOT_FOUND,
1344+
subcode=METADATA_COLUMN_NOT_FOUND,
13341345
)
13351346

13361347
attr_metadata_id = attr_meta.get("MetadataId")
@@ -1372,7 +1383,7 @@ def _flush_cache(
13721383
if k != "picklist":
13731384
raise ValidationError(
13741385
f"Unsupported cache kind '{kind}' (only 'picklist' is implemented)",
1375-
subcode=ec.VALIDATION_UNSUPPORTED_CACHE_KIND,
1386+
subcode=VALIDATION_UNSUPPORTED_CACHE_KIND,
13761387
)
13771388

13781389
removed = len(self._picklist_label_cache)

src/PowerPlatform/Dataverse/utils/__init__.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@
55
Utilities and adapters for the Dataverse SDK.
66
77
This module contains adapters (like Pandas integration).
8-
9-
Import classes directly from their specific modules:
10-
- Pandas integration: from .pandas_adapter import PandasODataClient
118
"""
129

13-
# No re-exports to avoid documentation duplication with py2docfx
1410
__all__ = []

tests/unit/core/test_http_errors.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import pytest
55
from PowerPlatform.Dataverse.core.errors import HttpError
6-
from PowerPlatform.Dataverse.core import error_codes as ec
6+
from PowerPlatform.Dataverse.core.error_codes import HTTP_404, HTTP_429, HTTP_500
77
from PowerPlatform.Dataverse.data.odata import ODataClient
88

99
class DummyAuth:
@@ -34,7 +34,7 @@ def json_fail(): raise ValueError("non-json")
3434
r.json = json_fail
3535
return r
3636

37-
class TestClient(ODataClient):
37+
class MockClient(ODataClient):
3838
def __init__(self, responses):
3939
super().__init__(DummyAuth(), "https://org.example", None)
4040
self._http = DummyHTTP(responses)
@@ -47,11 +47,11 @@ def test_http_404_subcode_and_service_code():
4747
{"x-ms-correlation-request-id": "cid1"},
4848
{"error": {"code": "0x800404", "message": "Not found"}},
4949
)]
50-
c = TestClient(responses)
50+
c = MockClient(responses)
5151
with pytest.raises(HttpError) as ei:
5252
c._request("get", c.api + "/accounts(abc)")
5353
err = ei.value.to_dict()
54-
assert err["subcode"] == ec.HTTP_404
54+
assert err["subcode"] == HTTP_404
5555
assert err["details"]["service_error_code"] == "0x800404"
5656

5757

@@ -61,12 +61,12 @@ def test_http_429_transient_and_retry_after():
6161
{"Retry-After": "7"},
6262
{"error": {"message": "Throttle"}},
6363
)]
64-
c = TestClient(responses)
64+
c = MockClient(responses)
6565
with pytest.raises(HttpError) as ei:
6666
c._request("get", c.api + "/accounts")
6767
err = ei.value.to_dict()
6868
assert err["is_transient"] is True
69-
assert err["subcode"] == ec.HTTP_429
69+
assert err["subcode"] == HTTP_429
7070
assert err["details"]["retry_after"] == 7
7171

7272

@@ -76,11 +76,11 @@ def test_http_500_body_excerpt():
7676
{},
7777
"Internal failure XYZ stack truncated",
7878
)]
79-
c = TestClient(responses)
79+
c = MockClient(responses)
8080
with pytest.raises(HttpError) as ei:
8181
c._request("get", c.api + "/accounts")
8282
err = ei.value.to_dict()
83-
assert err["subcode"] == ec.HTTP_500
83+
assert err["subcode"] == HTTP_500
8484
assert "XYZ stack" in err["details"]["body_excerpt"]
8585

8686

@@ -90,7 +90,7 @@ def test_http_non_mapped_status_code_subcode_fallback():
9090
{},
9191
{"error": {"message": "Teapot"}},
9292
)]
93-
c = TestClient(responses)
93+
c = MockClient(responses)
9494
with pytest.raises(HttpError) as ei:
9595
c._request("get", c.api + "/accounts")
9696
err = ei.value.to_dict()

tests/unit/data/test_logical_crud.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def json_func():
3434
resp.json = json_func
3535
return resp
3636

37-
class TestableClient(ODataClient):
37+
class MockableClient(ODataClient):
3838
def __init__(self, responses):
3939
super().__init__(DummyAuth(), "https://org.example", None)
4040
self._http = DummyHTTPClient(responses)
@@ -76,7 +76,7 @@ def test_single_create_update_delete_get():
7676
(204, {}, {}), # update (no body)
7777
(204, {}, {}), # delete
7878
]
79-
c = TestableClient(responses)
79+
c = MockableClient(responses)
8080
entity_set = c._entity_set_from_logical("account")
8181
rid = c._create(entity_set, "account", {"name": "Acme"})
8282
assert rid == guid
@@ -96,7 +96,7 @@ def test_bulk_create_and_update():
9696
(204, {}, {}), # UpdateMultiple broadcast
9797
(204, {}, {}), # UpdateMultiple 1:1
9898
]
99-
c = TestableClient(responses)
99+
c = MockableClient(responses)
100100
entity_set = c._entity_set_from_logical("account")
101101
ids = c._create_multiple(entity_set, "account", [{"name": "A"}, {"name": "B"}])
102102
assert ids == [g1, g2]
@@ -111,7 +111,7 @@ def test_get_multiple_paging():
111111
(200, {}, {"value": [{"accountid": "1"}], "@odata.nextLink": "https://org.example/api/data/v9.2/accounts?$skip=1"}),
112112
(200, {}, {"value": [{"accountid": "2"}]}),
113113
]
114-
c = TestableClient(responses)
114+
c = MockableClient(responses)
115115
pages = list(c._get_multiple("account", select=["accountid"], page_size=1))
116116
assert pages == [[{"accountid": "1"}], [{"accountid": "2"}]]
117117

@@ -120,6 +120,6 @@ def test_unknown_logical_name_raises():
120120
responses = [
121121
(200, {}, {"value": []}), # metadata lookup returns empty
122122
]
123-
c = TestableClient(responses)
123+
c = MockableClient(responses)
124124
with pytest.raises(MetadataError):
125125
c._entity_set_from_logical("nonexistent")

0 commit comments

Comments
 (0)