Refactor: extract pure-logic base classes to prepare for async implementation#167
Open
abelmilash-msft wants to merge 28 commits into
Open
Refactor: extract pure-logic base classes to prepare for async implementation#167abelmilash-msft wants to merge 28 commits into
abelmilash-msft wants to merge 28 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Extracts shared, I/O-free logic from the sync Dataverse OData and batch clients into new base classes to enable upcoming async client implementation while reducing duplication.
Changes:
- Added
_ODataBasewith URL/payload builders, caches, SQL guardrails, and lifecycle teardown. - Added
_BatchBasewith batch intent types plus multipart serialization/response parsing utilities. - Updated
_ODataClientand_BatchClientto inherit from the new base classes and adjusted a unit test patch target accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/data/test_sql_parse.py | Updates patch target to match the new urlparse location in _odata_base. |
| src/PowerPlatform/Dataverse/data/_odata_base.py | Introduces _ODataBase and shared helpers formerly in _odata.py. |
| src/PowerPlatform/Dataverse/data/_odata.py | Refactors _ODataClient to inherit shared “pure logic” from _ODataBase. |
| src/PowerPlatform/Dataverse/data/_batch_base.py | Introduces _BatchBase and moves intent types + multipart logic out of _batch.py. |
| src/PowerPlatform/Dataverse/data/_batch.py | Refactors _BatchClient to inherit batch serialization/parsing from _BatchBase. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…o v1 - Implemented unit tests for the `list_pages` method in `TestListPages` class, covering various scenarios including iterator return, page content validation, and parameter passing. - Added checks for deprecation warnings to ensure no warnings are raised during the usage of `list_pages`. - Introduced a new migration script `migrate_v0_to_v1.py` to automate the transition from beta (v0) to GA (v1) API calls, including method renaming and argument adjustments. - Created a new `tools` directory to house the migration script.
…pdate deprecated methods - Added simple and advanced streaming options in SKILL.md for records.list_pages() and execute_pages(). - Updated QueryBuilder to replace records.get() with records.list() in documentation and method calls. - Improved unit tests to validate new streaming functionality and ensure correct method delegation.
- Changed method name from `fetch_xml` to `fetchxml` across the codebase for consistency. - Updated relevant documentation to reflect the new method name. - Added a new example script for FetchXML usage demonstrating various scenarios. - Adjusted unit tests to accommodate the method name change and ensure proper functionality.
…aging improvements
vrathee-msft
previously approved these changes
May 6, 2026
…f QueryResult indexing
…ovals; modify prodev_quick_start.py for additional parameter; enhance pyproject.toml for migration tool; refine migrate_v0_to_v1.py documentation and usage instructions.
…ng and warnings; update migration tool for client variable support and manual review detection.
…agination options - Added `include_annotations` parameter to `_RecordGet` and `_RecordList` classes for OData requests. - Updated `_BatchClient` to handle new parameters in batch operations. - Enhanced `_ODataClient` methods to support `include_annotations`, `expand`, `page_size`, and `count` parameters. - Modified `BatchRecordOperations` to pass new parameters in batch record retrieval and listing methods. - Updated `RecordOperations` to include new parameters for retrieving and listing records. - Added unit tests to validate the new functionality for batch operations and record retrieval. - Implemented migration tool updates to handle changes in method signatures and ensure backward compatibility.
Extract all I/O-free methods from _ODataClient and _BatchClient into new shared base classes (_ODataBase, _BatchBase) so that a future async sibling can inherit the same pure logic without duplicating code. - Add data/_odata_base.py: _ODataBase with URL builders, payload constructors, cache helpers, _RequestContext, _USER_AGENT, _DEFAULT_EXPECTED_STATUSES, _extract_pagingcookie, _GUID_RE, _CALL_SCOPE_CORRELATION_ID, and _http_logger initialisation - Add data/_batch_base.py: _BatchBase with all intent dataclasses, multipart serialisation, response parsing, and the pure table resolvers - _ODataClient and _BatchClient now inherit from the respective base classes and retain only the I/O-dependent methods - Update test patch target for urlparse to _odata_base module - All 1223 unit tests pass; overall coverage remains at 94% Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The async batch client will pass an async OData client that inherits from _ODataBase, not _ODataClient. _BatchBase only calls pure _build_* methods defined on _ODataBase, so the broader base type is correct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add _ODataBase.close() to own cache clearing and logger teardown; _ODataClient.close() now delegates via super() then closes _http only - Reorder _ODataClient bases to (_FileUploadMixin, _RelationshipOperationsMixin, _ODataBase) so mixins are searched before the base, matching Python MRO convention Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…base Helpers moved to _BatchBase (_CRLF, _BOUNDARY_RE, _extract_boundary, etc.) are no longer re-imported by _BatchClient. Two test files updated to import these helpers directly from _batch_base where they now live. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… missing imports to _ODataBase - Remove _SQL_* patterns and _sql_guardrails from _ODataClient (now inherited from _ODataBase) - Add warnings import to _odata_base.py (needed by _sql_guardrails) - Add VALIDATION_SQL_WRITE_BLOCKED and VALIDATION_SQL_UNSUPPORTED_SYNTAX imports to _odata_base.py - Add missing table name lowercasing logic to _ODataBase._build_lookup_field_models
abelmilash-msft
force-pushed
the
users/abelmilash/async-phase1-shared-base
branch
from
4bf9eb8 to
656ed26
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add _BatchContext Protocol to _batch_base.py; re-type BatchRecordOperations, BatchTableOperations, BatchQueryOperations, BatchDataFrameOperations __init__ from BatchRequest to _BatchContext — removes type: ignore on AsyncBatchRequest - Extract _QueryBuilderBase from QueryBuilder with all fluent methods and build(); QueryBuilder inherits base and keeps execute/execute_pages/to_dataframe; AsyncQueryBuilder will inherit base directly, eliminating deprecated sync surface Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… just circular import avoidance Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both sync and async clients had identical 2-line initialization: ctx_obj = self.config.operation_context self._operation_context = ctx_obj.user_agent_context if ctx_obj else None Since self.config is already available in _ODataBase.__init__, this belongs in the base class. Subclasses inherit it and use it when building the User-Agent header. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
abelmilash-msft
force-pushed
the
users/abelmilash/async-phase1-shared-base
branch
from
4c7e524 to
f84ec5e
Compare
Self (from typing, under TYPE_CHECKING) is the idiomatic replacement for the self-referential TypeVar pattern. With `from __future__ import annotations` already in place, Self is never evaluated at runtime — only used by type checkers. Method signatures in docs now show `Self` instead of the private `_QB` TypeVar. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
abelmilash-msft
force-pushed
the
users/abelmilash/async-phase1-shared-base
branch
from
f84ec5e to
a83a688
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts pure logic (URL building, payload construction, parsing, caches, multipart serialization) from the sync clients into shared base classes
_ODataBaseand_BatchBase. The sync clients now inherit from these bases. The async client (Phase 2) will inherit from the same bases as a sibling, enabling code sharing without the async client being a subtype of the sync client._ODataBase— pure logic extracted from_ODataClient; owns cache teardown and HTTP logger lifecycle viaclose()_BatchBase— pure logic extracted from_BatchClient; owns multipart serialization and response parsingTests
1369 passed
Total coverage: 94.35% (threshold: 90%)