Skip to content

Add connectors: Azure Automation, OneNote, Key Vault, and more#37

Merged
hallvictoria merged 8 commits into
mainfrom
hallvictoria/add-new-connectors3
Jun 25, 2026
Merged

Add connectors: Azure Automation, OneNote, Key Vault, and more#37
hallvictoria merged 8 commits into
mainfrom
hallvictoria/add-new-connectors3

Conversation

@hallvictoria

@hallvictoria hallvictoria commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR adds 10 new connector clients to the Azure Connectors Python SDK, bringing the total validated connectors to 32. Each connector includes full unit test coverage and sample usage files.

Connector Module Tests Sample
Azure Automation azureautomation.py 32 tests
Azure Data Factory azuredatafactory.py 38 tests
Azure Digital Twins azuredigitaltwins.py 44 tests
Azure Key Vault keyvault.py 42 tests
Azure VM azurevm.py 43 tests
Microsoft Bookings microsoftbookings.py 36 tests
Office 365 Groups office365groups.py 50 tests
OneNote onenote.py 60 tests
Planner planner.py 66 tests
Yammer yammer.py 38 tests

Changes

  • 10 new connector client implementations in connectors
  • 10 new unit test files in tests (~449 new tests)
  • 10 new sample usage files in sample_connector_usage
  • Updated init.py exports for all new connectors
  • Updated documentation: README.md, CHANGELOG.md, SKILL.md files

Testing

  • Unit tests added/updated
  • All existing tests pass (pytest)
  • Linting passes (flake8 src/ tests/)
  • Manual testing (describe below if applicable)

Checklist

  • Code follows the project's coding conventions
  • No modifications to generated connector files in src/azure/connectors/*.py (unless regenerated)
  • Type hints added to all public API signatures
  • Documentation updated (if behavior changed)
  • CHANGELOG.md updated (if user-facing change)

@hallvictoria hallvictoria marked this pull request as ready for review June 9, 2026 16:32
@hallvictoria hallvictoria requested a review from a team as a code owner June 9, 2026 16:32
@daviburg

daviburg commented Jun 9, 2026

Copy link
Copy Markdown
Member

[Dobby] Finding 3 - Medium - Tests: newly generated Teams methods still covered only by skipped/pass-only tests

This PR adds 17 new async methods to teams.py (up from 11 on main). Several newly generated methods - get_channels_for_group_async (L2062), create_channel_async (L2091), get_all_channels_for_team_async (L2121), get_tags_async (L2626), create_tag_async (L2654) - have corresponding test stubs in test_teams.py that remain @pytest.mark.skip with empty pass bodies (L238-L286).

The skip reason states "Method has template variable bug - groupId not defined", but the PR regeneration now provides group_id: str as a proper parameter on these methods. If the generator fixed the template-variable bug, the skips are stale and should be replaced with active tests. If the bug persists in a different form, the skip reason should be updated to explain the current state.

Sibling sweep - 18 skipped tests total in test_teams.py (L238-L364):

  • Channel operations: 4 skipped (L238, L244, L250, L256)
  • Chat operations: 1 skipped (L266)
  • Tag operations: 6 skipped (L276, L282, L288, L294, L300, L306)
  • Message operations: 4 skipped (L316, L322, L328, L334)
  • Member operations: 1 skipped (L344)
  • Trigger operations: 2 skipped (L354, L360)

These 18 skipped tests were pre-existing on main, so they are not new regressions. However, this PR is the right place to unskip the ones whose generator bugs were fixed by the regeneration, since the methods now accept the parameters they were missing.

Additionally, no tests cover several other newly added methods that have no corresponding test stubs at all: get_team_async, post_message_to_self_async, list_team_members_async, add_member_to_team_async, remove_member_from_team_async, get_online_meeting_async, list_sections_async, create_section_async, get_all_adhoc_call_recordings_async, get_all_adhoc_call_transcripts_async.

Expected fix: unskip and implement active tests for the methods whose parameters now exist; add test stubs (at minimum) for the remaining newly generated methods.

@daviburg

daviburg commented Jun 9, 2026

Copy link
Copy Markdown
Member

[Dobby] Finding 4 - Low - Docs: CHANGELOG missing Yammer and has malformed version heading

The [Unreleased] section in CHANGELOG.md (L12-L20) lists nine of the ten new connectors added by this PR but omits Yammer (yammer.py). The README.md validated connectors table, samples/sample_connector_usage/README.md, and .github/skills/connection-setup/SKILL.md all correctly include Yammer, so the CHANGELOG is the only out-of-sync surface.

Additionally, the heading for the previous release at L22 reads ## [0.3.b2] but the reference link at L123 reads [0.3.0b2]. The heading and the link label must match for the Keep a Changelog link to resolve. Based on the git tag convention (v0.3.0b2), the heading should be ## [0.3.0b2].

Sibling sweep across doc surfaces:

Surface Yammer present? Version label correct?
CHANGELOG.md [Unreleased] (L12-L20) Missing N/A
CHANGELOG.md release heading (L22) N/A 0.3.b2 (wrong)
CHANGELOG.md reference link (L123) N/A 0.3.0b2 (correct)
README.md validated table (L188) Present N/A
samples README.md (L52) Present N/A
connection-setup SKILL.md (L68) Present N/A
init.py exports Present N/A

Expected fix:

  1. Add - **Yammer** (yammer.py) connector client with unit tests and samples to the [Unreleased] section, alphabetically.
  2. Change L22 from ## [0.3.b2] to ## [0.3.0b2].

@daviburg

daviburg commented Jun 9, 2026

Copy link
Copy Markdown
Member

[Dobby] Finding 1 - High - Contract: Azure Digital Twins add/update methods cannot send request payloads

Six ADT methods expose no input parameter and send body=None, making the operations nonfunctional for callers who need to create or update resources.

Python SDK (this PR):

  • add_twin_async(self, twinid: str) at azuredigitaltwins.py L375 - sends body=None at L386
  • add_relationship_async(self, twinid, relationship_id) at L508 - sends body=None at L523
  • update_model_async(self, modelid) at L322 - sends body=None at L333
  • update_twin_async(self, twinid) at L401 - sends body=None at L412
  • update_component_async(self, twinid, component_path) at L444 - sends body=None at L459
  • update_relationship_async(self, twinid, relationship_id) at L538 - sends body=None at L553

Comparison with .NET SDK (AzureDigitalTwinsExtensions.cs on main):

Operation .NET SDK signature Python SDK signature
AddTwin (L834) AddTwinAsync(string digitalTwinId, AddTwinInput input, ...) add_twin_async(self, twinid) - no input
AddRelationship (L1021) AddRelationshipAsync(string digitalTwinId, string relationshipId, AddRelationshipInput input, ...) add_relationship_async(self, twinid, relationship_id) - no input
UpdateModel (L747) UpdateModelAsync(string modelId, UpdateModelInput input, ...) update_model_async(self, modelid) - no input
UpdateTwin (L863) UpdateTwinAsync(string digitalTwinId, UpdateTwinInput input, ...) update_twin_async(self, twinid) - no input
UpdateComponent (L925) UpdateComponentAsync(string digitalTwinId, string dTDLComponent, UpdateComponentInput input, ...) update_component_async(self, twinid, component_path) - no input
UpdateRelationship (L1053) UpdateRelationshipAsync(string digitalTwinId, string relationshipId, UpdateRelationshipInput input, ...) update_relationship_async(self, twinid, relationship_id) - no input

Every .NET method accepts a typed input body (AddTwinInput, UpdateTwinInput, etc.). The Python generator dropped these parameters. The AddModelsInput parameter was correctly generated (L206), showing the generator can emit input parameters - it just missed these six.

Expected fix: regenerate or manually add the missing input parameters (with corresponding dataclasses) and pass them to send_async. Update the existing tests to assert the outgoing request body, not only method and path.

@daviburg

daviburg commented Jun 9, 2026

Copy link
Copy Markdown
Member

[Dobby] Finding 2 - High - Error Handling: void operations silently swallow server errors

Many generated DELETE, PATCH, and fire-and-forget POST methods call await self.http_client.send_async(...) without capturing or checking the response. Because the Python SDK's send_async (http_client.py L176) explicitly returns the response snapshot for callers to handle (unlike the .NET SDK which auto-throws), a 400/401/404/500 from these operations looks like success to SDK callers.

Comparison with .NET SDK (ConnectorClientBase.cs on main):

The .NET SDK has centralized auto-throw in two base-class helpers:

  • CallConnectorAsync<TResponse>() at L136: checks response.IsError (L164), throws ConnectorException (L167)
  • CallConnectorAsync() at L193 (void overload): same check (L221-L224)

Every generated .NET method - including DeleteModelAsync, DeleteTwinAsync, SendTelemetryAsync - calls through the base class and gets automatic error reporting for all HTTP verbs.

The Python SDK chose a different (valid) architecture: send_async returns the raw snapshot and each generated method is responsible for status checking. The GET/POST-with-return methods do this correctly. The void methods do not. This is a generator bug, not a design disagreement.

Sibling sweep - 37 unchecked send_async calls across 9 changed connector files:

File Unchecked calls Operations
azuredigitaltwins.py 9 DELETE model/twin/relationship, PATCH model/twin/component/relationship, POST telemetry x2
azurevm.py 12 POST start/stop/restart/deallocate/poweroff/redeploy/reimage for VMs and scale set VMs
teams.py 6 POST triggers, DELETE tag/member
office365groups.py 4 POST/DELETE member operations
microsoftbookings.py 2 PATCH update, DELETE cancel
azuredatafactory.py 1 POST cancel pipeline
onenote.py 1 DELETE page
planner.py 1 DELETE task
yammer.py 1 POST like message

Note: this same gap also exists in some pre-existing connectors on main (azurequeues, azuretables, commondataservice, eventhubs, excelonlinebusiness, office365), indicating a systemic generator issue. Fixing it in this PR for the changed files would be ideal; a follow-up issue for the rest is acceptable.

Expected fix: for every await self.http_client.send_async(...) that discards the response, capture the response and add the standard status check:
python response = await self.http_client.send_async(method, path, body=...) if not (200 <= response.status < 300): raise ConnectorException(method, path, response.status, response.text)
Add negative-path tests for at least one representative DELETE, PATCH, and fire-and-forget POST per connector.

@daviburg daviburg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Dobby] Review of PR #37 - Add connectors: Azure Automation, OneNote, Key Vault, and more

Solid batch of 10 new connector clients with tests, samples, docs fan-out, and passing CI across Python 3.10-3.14. Two issues need fixes before merge; two are lower-priority improvements.

Must fix (High):

  1. ADT add/update methods missing input payloads - 6 Azure Digital Twins methods (add_twin, add_relationship, update_model, update_twin, update_component, update_relationship) send body=None, making them nonfunctional. The .NET SDK's equivalent methods all accept typed input bodies. This is a code generation gap.

  2. Void operations silently swallow server errors - 37 calls across 9 changed connector files discard the HTTP response without checking status. The .NET SDK auto-throws on non-2xx via its base class; the Python SDK's architecture requires each generated method to check, but void methods skip this. Callers cannot distinguish success from 400/401/404/500.

Should fix (Medium/Low):

  1. Teams skipped tests may be stale - 18 tests remain skipped with 'template variable bug' but the PR regeneration appears to have added the missing parameters. If fixed, unskip; if not, update the skip reason. Also, 10+ newly added Teams methods have no test stubs at all.

  2. CHANGELOG missing Yammer + malformed version heading - Yammer is in README/samples/SKILL but not CHANGELOG. The heading '## [0.3.b2]' doesn't match its reference link '[0.3.0b2]'.

See individual comments for detailed evidence, .NET SDK comparison tables, and sibling sweep results.

@daviburg

daviburg commented Jun 9, 2026

Copy link
Copy Markdown
Member

[Dobby] Agent tooling / generation-guardrail suggestions (process, not blocking)

While reviewing this PR I looked at whether the repo's agent tooling has what's needed to produce clean generated clients. The docs cover how to run the generator (GENERATION.md) and how to wire up its output (add-connector skill, connector-table instructions), and they state the right principle - fix the generator, not the generated file (GENERATION.md Key Rules). What's missing is the validation layer that would have caught Findings 1 and 2 before merge. Both High findings are predictable symptoms of that gap, since CI was fully green.

Suggested improvements:

  1. Define "clean generated client" acceptance criteria. Add a section to GENERATION.md (or a new connector-generation.instructions.md) enumerating the invariants every generated client must satisfy:

    • every create/update operation accepts and sends a request body (Finding 1)
    • every operation - including void DELETE/PATCH/POST - captures the response and raises ConnectorException on non-2xx (Finding 2)
    • no skipped tests without a tracked issue link (Finding 3)
  2. Strengthen the add-connector test template. The current per-method template only asserts success/error for methods that return a value. It should also assert (a) the outgoing request body for create/update methods, and (b) that void methods raise on non-2xx. With those assertions, a generator that drops an input param or a status check would fail tests instead of passing - the exact gap that let Findings 1 and 2 through.

  3. Add a CI lint/AST check for discarded responses. A small check that flags any await self.http_client.send_async(...) whose result is not assigned and status-checked would have caught all 37 sibling occurrences in Finding 2 automatically, plus the pre-existing ones in azurequeues/azuretables/commondataservice/eventhubs.

  4. Point the "fix the generator" loop at the actual Python emitter. GENERATION.md says fix the generator but never names the files. The Python emitter lives in the BPM repo at src/tools/CodefulSdkGenerator/DirectClient/DirectClientPythonGenerator.cs and DirectClientPythonCodeGenerator.cs. Notably, the .NET generator centralizes error handling in ConnectorClientBase.CallConnectorAsync() (auto-throws on non-2xx for void ops too); the Python emitter does not mirror this, which is the root cause of Finding 2. Documenting this divergence and the file pointers would make the upstream fix actionable.

  5. Add a known-generator-defects registry. The Teams template variable bug skips (Finding 3) have shipped across multiple releases with no tracking. A short registry in GENERATION.md (defect -> BPM issue link -> affected connectors) would prevent silent recurrence; GENERATION.md currently just says report issues to BPM team with no link or owner.

These are process suggestions for the SDK/generator maintainers and are not blockers for this PR beyond the already-filed Findings 1-4.

@hallvictoria

Copy link
Copy Markdown
Collaborator Author

Addressing Feedback:

  1. Finding 1: patched SDK Generator and regenerated azuredigitaltwins.py file.

    • Added api_version parameter to all method calls
    • Updated add_twin_async/add_relationship_async to use new input types
    • Updated test_azuredigitaltwins.py for connector regeneration
  2. Finding 2: addressed issue in SDK Generator and regenerated all impacted connector files.

  3. Finding 3: Addressed skipped tests for regenerated methods in test_teams.py

    • Unskipped and implemented 7 tests (channel/tag operations now have proper group_id parameters)
    • Added 17 new tests for newly generated methods (team, meeting, section, ad-hoc call operations)
    • Updated skip reasons for methods that don't exist in SDK
  4. Finding 4: documentation fixes

    • Added missing Yammer connector to [Unreleased] section
    • Fixed version heading: ## [0.3.b2] → ## [0.3.0b2]
  5. Agent tooling / generation-guardrail suggestions: created follow-up issue: Agent tooling / generation-guardrail suggestions #39

@hallvictoria hallvictoria requested a review from daviburg June 9, 2026 20:49

@daviburg daviburg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Dobby] Re-review after f6e7c46: Findings 1, 2, and 4 look fixed, but Finding 3 is still not fully resolved, so I cannot sign off yet.

The remaining problem is that tests/test_teams.py still skips tests with reasons saying the methods do not exist in the generated SDK, but those methods now do exist in src/azure/connectors/teams.py. This means CI can stay green while these generated operations remain untested.

Examples from the current PR head:

Skipped test Skip reason Generated method exists
test_get_channel_success at tests/test_teams.py:297 Method get_channel_async does not exist in generated SDK get_channel_async at src/azure/connectors/teams.py:2136
test_get_chats_success at tests/test_teams.py:333 Method get_chats_async does not exist in generated SDK get_chats_async at src/azure/connectors/teams.py:2211
test_add_member_to_tag_success at tests/test_teams.py:423 Method add_member_to_tag_async does not exist in generated SDK add_member_to_tag_async at src/azure/connectors/teams.py:4237
test_get_tag_members_success at tests/test_teams.py:429 Method get_tag_members_async does not exist in generated SDK get_tag_members_async at src/azure/connectors/teams.py:4268
test_delete_tag_member_success at tests/test_teams.py:435 Method delete_tag_member_async does not exist in generated SDK delete_tag_member_async at src/azure/connectors/teams.py:4298
test_delete_tag_success at tests/test_teams.py:441 Method delete_tag_async does not exist in generated SDK delete_tag_async at src/azure/connectors/teams.py:4212
test_get_message_details_success at tests/test_teams.py:457 Method get_message_details_async does not exist in generated SDK get_message_details_async at src/azure/connectors/teams.py:2344
test_list_replies_to_message_success / test_list_replies_with_top_parameter at tests/test_teams.py:463 and :469 Method list_replies_to_message_async does not exist in generated SDK list_replies_to_message_async at src/azure/connectors/teams.py:2380
test_list_members_success at tests/test_teams.py:506 Method list_members_async does not exist in generated SDK list_members_async at src/azure/connectors/teams.py:2429
test_on_new_channel_message_success / test_on_new_channel_message_with_top at tests/test_teams.py:587 and :593 Method on_new_channel_message_async does not exist in generated SDK on_new_channel_message_async at src/azure/connectors/teams.py:2468

There is also test_get_messages_from_channel_success at tests/test_teams.py:451 with the generic reason Method does not exist in generated SDK, while get_messages_from_channel_async exists in teams.py.

Please unskip and implement these now that the methods are generated, or remove/rename the stale tests if a different operation is intended. After that, I think this should be ready: ADT request bodies are present, changed connector files no longer discard send_async responses, Yammer/changelog docs are fixed, and CI is green.

Local note: I could not run the targeted pytest files locally because this machine is missing aiohttp, but the current PR CI passed across Python 3.10-3.14.

Comment thread src/azure/connectors/onenote.py
Comment thread src/azure/connectors/onenote.py
Comment thread src/azure/connectors/onenote.py
Comment thread tests/test_azureautomation.py
Comment thread tests/test_teams.py Outdated
Comment thread samples/sample_connector_usage/sample_connector_usage_azureautomation.py Outdated
Comment thread src/azure/connectors/azuredigitaltwins.py Outdated
Comment thread samples/sample_connector_usage/sample_connector_usage_planner.py Outdated
@hallvictoria hallvictoria requested a review from a team as a code owner June 24, 2026 21:02
Comment thread samples/sample_connector_usage/sample_connector_usage_keyvault.py Fixed
Comment thread samples/sample_connector_usage/sample_connector_usage_keyvault.py Fixed
Comment thread samples/sample_connector_usage/sample_connector_usage_keyvault.py Fixed
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

@daviburg daviburg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now!

@hallvictoria hallvictoria merged commit 8f9cdd9 into main Jun 25, 2026
17 checks passed
@hallvictoria hallvictoria deleted the hallvictoria/add-new-connectors3 branch June 25, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants