You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Regenerate Python connectors from merged query-contract fixes - #101
Regenerate the existing 99-connector Python inventory and show the downstream result of the paired generator fix. The 33-client internal-default encoding delta and its exact SDK regression are published in commit dab5d97. No generated file was hand-edited.
Paired Generator Source
BPM PR 17368905, CodefulSdkGenerator: encode internal Python query defaults, by David Burg.
Exact generating source: commit 71906fe, rebuilt compiler version 1.193.0.9.
Status: active/unmerged. This SDK PR intentionally shows that linked BPM PR's generated outcome before upstream merge. Publication is complete; upstream integration remains a separate final SDK merge gate. The source is not described as merged.
The generator suite passed 821 tests, including old-fails/new-passes short and wrapped internal-default regressions.
Current Generated Outcome
Caller-supplied and internal-default query values escape reserved characters, including /.
The committed Groups Mail assertion now checks $filter=groupTypes%2Fany%28c%3Ac%20eq%20%27Unified%27%29 exactly and passes. It is no longer a local, uncommitted, failing test.
All 99 clients were generated from fresh live westus definitions in subscription f34b22a3-2202-4fb1-b040-1332bd928c84, API 2018-07-01-preview, using a new empty temporary response directory. Temporary inputs/outputs are removed after validation; reviewers make independent live downloads.
Jira's October 6 live fields: Optional[str] contract is retained. Its tests cover absent, explicit, and empty fields plus named continuation-token binding; callers should use keyword arguments for fields/token.
Seismic Planner tests execute CSV IDs and the existing runtime scalar fallback with slash-bearing values, plus empty/omitted controls. Both slash assertions fail against the immutable old client and pass now.
Paging and Consumer Changes
The existing automatic item-iterator contract is propagated to 32 additional connectors. This is separate from query encoding and traces to merged BPM PR 17086991, CodefulSdkGenerator: preserve Dataverse pagination continuations, by David Burg, and Python PR #84, Fix Dataverse list pagination, by David Burg (@daviburg).
Changed samples capture semantic collections directly, preserve nullable non-pageable guards, and are tested with empty/nonempty/None results as appropriate. Actual continuation tests assert later-page requests/items and final termination, including an empty first page. Scope/consumer findings have contextual replies and evidence; no cache/replay scaffolding was introduced.
Validation
Full current Python suite: 4,279 passed, three warnings, zero failures.
Full source/test/sample lint and source mypy passed (108 source files).
Focused Groups Mail/Jira/Seismic Planner checks: 70 passed. The exact internal-default filter check passes against the paired output.
Generator suite: 821 passed; source-linked paired output is ready for review, not inferred to be merged upstream.
The TypeScript sibling was generated independently from the same active source with fresh live inputs: all 75 committed outputs match; no TypeScript generated-code churn is needed for this Python-specific fix.
No live connector actions or deployed samples were exercised.
Remaining Review Gates
Event Hubs scalar-map compatibility remains an open, separate contract decision in its individual thread. This encoding change does not resolve it.
Final SDK merge requires upstream integration and the remaining human review/contract gates. It does not block showing paired generated output here.
Companion TypeScript PR #105 records the unchanged sibling result and the same paired-publication workflow.
Populated Pagination and Validator Coverage
Commit dabdac0closesthe concrete Copilot pagination request. Shared pageable success fixtures contain contract-keyed populated items. Elfsquad OData entities and all 11 pageable newly generated-operation cases assert populated multi-page results, empty-first-page continuations, exact later requests/items, and termination. Empty-body, empty-collection, and 204 controls remain separate. Wrong-key and missing-continuation mutations are rejected by nine negative controls without changing product files.
The same revision repairs RHS-before-reassignment and parameter-shadowing validator misses, including sync/async scopes and outer default evaluation. Focused connector/operation tests: 117 passed; sample/validator tests: 140 passed; full suite: 4,279 passed with three warnings. Lint and source typing pass. No live service execution, generated-client change, or resolution of the unrelated Event Hubs contract is implied by these test-infrastructure fixes.
Follow-Up Copilot Findings
Commit 15e3c3a addresses all three new inline findings with contextual replies and verified resolution:
Comprehensions now use local target facts across list/set/dict/generator forms, validate each iterable before binding, and restore enclosing facts. Five old-failing shadowing cases now pass; the invalid outer-iterable control remains detected. All 146 sample/validator tests pass.
Azure AD, Slack, and Azure Tables have their own populated/empty-first continuation tests using their exact next-link fields. Exact requests, bodies, yielded dictionaries, and termination are asserted while single-page controls remain. All 164 connector/Elfsquad tests pass; six wrong-continuation mutations are rejected without changing product files.
The 11 expected new pageable methods are declared independently of runtime inspection. Iterator shape, case retention, and populated fixtures are invariant. A single-page coroutine mutation retains all 11 cases but fails four guards; the shared success helper is also declaration-driven. All 102 operation-harness tests pass.
Full current Python validation: 4,279 passed, three warnings; lint and source typing pass. No generated file changed in this iteration. The user-authored workflow merge was fast-forwarded and preserved. The Event Hubs contract thread and separate TypeScript provenance findings are not represented as resolved by these focused repairs.
Regenerates the connector inventory with corrected query escaping and async-iterator pagination, alongside updated tests, samples, and provenance validation.
Changes:
Fully percent-encodes generated query values.
Converts pageable operations to async iterators that follow continuation links.
Updates tests, samples, and regeneration validation.
Automatic paging: yes, iteration follows continuation links automatically. test_success_with_pagination calls the real generated method and asserts both GET requests, the exact continuation token, second-page items, and termination, including an empty first page. This is execution coverage, not a substitute code comment. The item iterator is not claimed to expose Azure Core page metadata.
Semantic response capture: the sample now assigns subscriptions = [item async for item in client.subscriptions_list_async()] directly. Applied semantic captures and duplicate-condition removal across the changed samples, not a one-off rename. Both SDKs' active instructions carry the principle. I left your follow-up discussions open for confirmation and did not overwrite your comments.
Ask
Disposition and evidence
Automatic paging
Implemented; real two-request/empty-first-page assertions pass; open for confirmation.
Semantic captures
Implemented across changed consumers; negative/positive naming checks pass; open for confirmation.
Repeated conditions
Removed; negative items and items and positive distinct-name controls pass; replied/resolved.
Cosmos DB envelope reads
Removed from both samples; nonempty execution reproduces old failures and now passes; replied/resolved.
Capture directly in items; zero/two-item executions pass; replied/resolved.
Added cache/scaffolding
Entire new Python cache/manifest and new TypeScript catalog removed; artifact absence verified; replied/resolved.
Refresh from live
New empty temporary directories fetched 99 Python and 75 TypeScript definitions; every generated client matched; stores removed; replied/resolved.
Independent live review
Active instructions prohibit producer responses/fixtures as review inputs; separate live download required; replied/resolved.
Miss/prevention record: the original reviewed head a2dc1f2 passed import/signature checks but those checks did not execute response consumption. New tests reproduced four nonempty failures with four empty controls passing; after the consumer fixes, all eight pass. The validator also failed its new list-access regression before the repair and passed afterward. Runtime sample tests, valid counterexamples, and actual continuation assertions now provide reusable prevention; policy guidance alone does not guarantee agent compliance.
Validation: 4,174 Python tests, 118 sample tests, and 1,253 TypeScript tests passed; lint/type checks passed. No live connector actions or deployed samples were exercised. Hosted checks and a fresh review are tracked separately; this is not a blanket merge-ready claim.
The generated URL therefore contains groupTypes/any... rather than groupTypes%2Fany..., contradicting this PR's claim that reserved characters, including /, are escaped.
Please update the generator's internal-default query path to emit quote(default, safe=''), add a generator regression using a slash-bearing default, regenerate, and assert the exact encoded filter here.
[Dobby] Medium - Scope: link the paging provenance
CHANGELOG.md:17-19 documents the breaking paging rollout, but the PR's source section links only the query-contract generator PR. This paging behavior comes from separate work: BPM PR 17086991 and Python PR #84.
Please link those sources and state that this regeneration propagates the paging contract to 32 connectors. That makes the breaking change independently traceable instead of presenting it as part of the query repair.
This duplicates the identical check on line 194, so the inner else is unreachable and the sample has two competing empty-result messages. Keep a single branch around the loop.
Preserve scalar application-property values in response model
src/azure/connectors/eventhubs.py:34
This narrowing makes the public response model reject its established scalar application-property values: tests/test_eventhubs.py:335-342 constructs Event(properties={"type": "notification"}). Keep the dictionary value type broad enough for the actual scalar contract (as before), and fix the generator before regenerating this file.
Preserve scalar values for SendEvent properties
src/azure/connectors/eventhubs.py:100
SendEvent.properties is still used with string application properties throughout the supported API (tests/test_eventhubs.py:157-160,388-393 and the Event Hubs sample), so changing it to Dict[str, ObjectEntity] makes valid calls fail static type checking even though runtime serialization accepts them. Preserve the scalar/Any value contract in the generator and regenerate this client.
The async comprehension already always produces a list, so this self-assignment is a no-op. Remove it to keep the migrated consumer direct and avoid the redundant condition.
This issue also appears on line 72 of the same file.
The async comprehension already always produces a list, so this self-assignment is a no-op. Remove it to keep the migrated consumer direct and avoid the redundant condition.
This issue also appears on line 80 of the same file.
The async comprehension already always produces a list, so this self-assignment is a no-op. Remove it to keep the migrated consumer direct and avoid the redundant condition.
The async comprehension already always produces a list, so this self-assignment is a no-op. Remove it to keep the migrated consumer direct and avoid the redundant condition.
The async comprehension already always produces a list, so this self-assignment is a no-op. Remove it to keep the migrated consumer direct and avoid the redundant condition.
I rebuilt the declared merged BPM revision 7c26edf, compiler version 1.193.0.5, and generated only Jira from fresh westus definitions in subscription f34b22a3-2202-4fb1-b040-1332bd928c84, using a new empty response directory. A second independent empty-directory live download reproduced the new Python file exactly after line-ending normalization. Input/output stores were then removed.
list_issues_async now exposes fields: Optional[str] = None, omits fields when unspecified, and no longer injects fields=*all. The new signature/omitted/populated/empty tests all failed against the previous client and pass after regeneration; a named next_page_token test confirms its existing query binding. The changelog notes that callers should use keyword arguments because fields precedes the token in the generated signature. All 25 Jira tests pass.
I independently checked the TypeScript sibling: its fields option was already public, and today's output differs only in the live description. TypeScript commit a3f6f91 refreshes that exact generated documentation, its existing Jira fixture/hash entry, a per-Jira capture timestamp, and optional-fields request tests; filtered registries were not applied. All 9 TypeScript Jira tests and the full 1,260-test suite pass.
Disposition: fixed and independently verified for Jira. This is a narrow October 6 Jira refresh, not a claim that every other connector was freshly recaptured today. The separate unmerged internal-default encoding and Event Hubs contract blockers remain explicit; no output from the unmerged generator repair was used.
[Dobby] Updated in place: the paired generator output is now published
hallvictoria, correction to my earlier response to your internal-default follow-up: I incorrectly treated upstream merge as a prerequisite for publishing the downstream outcome. The generator and SDK PRs are a review pair.
SDK commit dab5d97 now contains all affected generated Python clients and the exact encoded-filter assertion, generated from BPM PR 17368905, CodefulSdkGenerator: encode internal Python query defaults, by David Burg, at exact source 71906fe, compiler 1.193.0.9.
That source is active/unmerged, and is deliberately identified as such. All 99 clients were generated from fresh live definitions, 33 clients changed for internal-default encoding, and the previously red assertion now passes with $filter=groupTypes%2Fany%28c%3Ac%20eq%20%27Unified%27%29. The full current Python suite passes 4,197 tests with zero failures; lint and source typing pass. The exact test is committed, not merely prepared locally.
The corresponding 75 TypeScript outputs were checked from the same source using a separate fresh live download and were byte-identical after line-ending normalization, so no generated TypeScript churn was needed. Both PRs' active guidance now distinguish showing paired output from final merge readiness. Temporary response/output stores are removed after verification.
Disposition: the encoding implementation and downstream regression are now published and passing. Upstream integration is still a final merge gate, not a publication blocker. The separate Event Hubs map-contract thread is unchanged and is not claimed fixed by this encoding result.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Unresolved schema contracts and pending generator integration require human review before approval.
Review effort: Balanced Findings: None
Previously missed (3)
In code that hasn't changed since last review
Validate RHS before updating reassigned variable type
scripts/validate_connector_samples.py:98
This clears the assignment target's type before validating the right-hand side. After collecting items, items = items.get('value', []) therefore passes validation even though it raises AttributeError; the standalone items.get(...) control is correctly rejected. Visit the right-hand side with the previous type facts before updating either target map, and add a same-name reassignment regression.
Clear shadowed parameter facts before function validation
scripts/validate_connector_samples.py:106
Outer type facts remain active for parameters that shadow those names. For example, items = [] at module scope followed by async def consume(items: dict) makes a valid items.get(...) call fail validation as a list method error. Clear inherited variable and client facts for parameter names before visiting the function, then restore the outer scope afterward. Add a shadowing regression alongside the existing validator tests.
Resolve Event Hubs scalar-map type compatibility
src/azure/connectors/eventhubs.py:100
Both SendEvent.properties here and Event.properties at line 34 now require ObjectEntity values. Existing tests construct scalar maps such as {"type": "greeting"} and {"type": "notification"} (tests/test_eventhubs.py:159,337), and the sample also sends string properties. These payloads still work with the dataclasses at runtime but no longer satisfy their public types. Complete the pending Event Hubs contract arbitration and approved schema normalization, then regenerate both models and add scalar-map compatibility coverage. Do not hand-edit the generated annotations or widen unrelated typed maps.
RHS before reassignment:visit_Assign visits the value with the existing type/client facts before updating targets, and clears stale client facts on non-client assignments. The new regression reproduces items = items.get('value', []): the old validator emitted no finding, whereas the repaired validator reports the list-method error. Existing standalone-access controls remain intact.
Parameter shadowing: sync and async functions clear inherited parameter facts for positional, positional-only, keyword-only, vararg, and kwarg names while visiting the body, then restore the enclosing maps. Tests exercise sync/async ordinary/positional-only/keyword-only shadowing and require the outer list check still to work afterward. Function defaults, annotations, and decorators are visited in the outer scope before shadowing; a further regression catches an invalid outer client method in a same-name default expression.
Old-fails/new-passes evidence: the same-name reassignment and initial sync/async shadowing cases all failed before the repair; the default-expression control also exposed a local false negative before its correction. All 140 existing and new sample/validator tests pass after the fixes. Full Python suite: 4,266 passed, three warnings; full lint and source typing passed.
The separate pagination finding has its own contextual reply and resolution, not this global substitute. The Event Hubs type-contract finding in the same review body remains separate and is not resolved by these validator changes. There are no inline threads for these two body-only validator asks; this response links their exact source and gives each its own action/evidence.
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
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
Regenerate the existing 99-connector Python inventory and show the downstream result of the paired generator fix. The 33-client internal-default encoding delta and its exact SDK regression are published in commit dab5d97. No generated file was hand-edited.
Paired Generator Source
1.193.0.9.Current Generated Outcome
/.$filter=groupTypes%2Fany%28c%3Ac%20eq%20%27Unified%27%29exactly and passes. It is no longer a local, uncommitted, failing test.f34b22a3-2202-4fb1-b040-1332bd928c84, API2018-07-01-preview, using a new empty temporary response directory. Temporary inputs/outputs are removed after validation; reviewers make independent live downloads.fields: Optional[str]contract is retained. Its tests cover absent, explicit, and empty fields plus named continuation-token binding; callers should use keyword arguments for fields/token.Paging and Consumer Changes
The existing automatic item-iterator contract is propagated to 32 additional connectors. This is separate from query encoding and traces to merged BPM PR 17086991, CodefulSdkGenerator: preserve Dataverse pagination continuations, by David Burg, and Python PR #84, Fix Dataverse list pagination, by David Burg (@daviburg).
Changed samples capture semantic collections directly, preserve nullable non-pageable guards, and are tested with empty/nonempty/None results as appropriate. Actual continuation tests assert later-page requests/items and final termination, including an empty first page. Scope/consumer findings have contextual replies and evidence; no cache/replay scaffolding was introduced.
Validation
Remaining Review Gates
Populated Pagination and Validator Coverage
Commit dabdac0 closes the concrete Copilot pagination request. Shared pageable success fixtures contain contract-keyed populated items. Elfsquad OData entities and all 11 pageable newly generated-operation cases assert populated multi-page results, empty-first-page continuations, exact later requests/items, and termination. Empty-body, empty-collection, and 204 controls remain separate. Wrong-key and missing-continuation mutations are rejected by nine negative controls without changing product files.
The same revision repairs RHS-before-reassignment and parameter-shadowing validator misses, including sync/async scopes and outer default evaluation. Focused connector/operation tests: 117 passed; sample/validator tests: 140 passed; full suite: 4,279 passed with three warnings. Lint and source typing pass. No live service execution, generated-client change, or resolution of the unrelated Event Hubs contract is implied by these test-infrastructure fixes.
Follow-Up Copilot Findings
Commit 15e3c3a addresses all three new inline findings with contextual replies and verified resolution:
Full current Python validation: 4,279 passed, three warnings; lint and source typing pass. No generated file changed in this iteration. The user-authored workflow merge was fast-forwarded and preserved. The Event Hubs contract thread and separate TypeScript provenance findings are not represented as resolved by these focused repairs.