Update samples for Azure Connectors SDK 0.14 - #72
Conversation
Co-authored-by: Dobby <dobby@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Updates the DirectConnector sample project to consume the latest Azure.Connectors.Sdk preview package, keeping the sample aligned with current generated connector clients.
Changes:
- Bumped
Azure.Connectors.Sdkfrom0.13.0-preview.1to0.14.0-preview.1in the DirectConnector project.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Dobby <dobby@microsoft.com>
Co-authored-by: Dobby <dobby@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (17)
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:44
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task AzureDigitalTwinsListModelsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new AzureDigitalTwinsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new AzureDigitalTwinsFunctions(TestHelpers.CreateNullLogger<AzureDigitalTwinsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListModelsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:54
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task AzureVMListSubscriptionsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new AzureVMClient(uri, credential, options),
"{\"value\":[]}");
var functions = new AzureVMFunctions(TestHelpers.CreateNullLogger<AzureVMFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListSubscriptionsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:64
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task KeyVaultListSecretsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new KeyVaultClient(uri, credential, options),
"{\"value\":[]}");
var functions = new KeyVaultFunctions(TestHelpers.CreateNullLogger<KeyVaultFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListSecretsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:74
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task MicrosoftBookingsListBusinessesAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new MicrosoftBookingsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new MicrosoftBookingsFunctions(TestHelpers.CreateNullLogger<MicrosoftBookingsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListBusinessesAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:84
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task Office365GroupsListGroupsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new Office365GroupsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new Office365GroupsFunctions(TestHelpers.CreateNullLogger<Office365GroupsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListGroupsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:94
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task Office365GroupsMailListGroupsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new Office365GroupsMailClient(uri, credential, options),
"{\"value\":[]}");
var functions = new Office365GroupsMailFunctions(TestHelpers.CreateNullLogger<Office365GroupsMailFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListGroupsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:104
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task OnenoteListNotebooksAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new OnenoteClient(uri, credential, options),
"[]");
var functions = new OnenoteFunctions(TestHelpers.CreateNullLogger<OnenoteFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListNotebooksAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:114
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task PowerBIListWorkspacesAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new PowerBIClient(uri, credential, options),
"{\"value\":[]}");
var functions = new PowerBIFunctions(TestHelpers.CreateNullLogger<PowerBIFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListWorkspacesAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:124
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task ShiftsListTeamsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new ShiftsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new ShiftsFunctions(TestHelpers.CreateNullLogger<ShiftsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListTeamsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:134
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client can be disposed before the async assertion completes. Make the testasyncandawaitthe assertion.
[TestMethod]
public Task TodoListTaskListsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new TodoClient(uri, credential, options),
"[]");
var functions = new TodoFunctions(TestHelpers.CreateNullLogger<TodoFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListTaskListsAsync);
}
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:34
- These test methods return an un-awaited Task while the mocked client is declared with
using var. The client (and underlying HttpClient pipeline) can be disposed beforeAssertSuccessAsync(...)completes, leading to flaky failures. Make the testasyncandawaitthe assertion so theusingscope lives for the whole async operation.
This issue also appears in the following locations of the same file:
- line 36
- line 46
- line 56
- line 66
- line 76
- ...and 5 more
[TestMethod]
public Task AzureAutomationListSubscriptionsAsync_WithValidResponse_ReturnsOk()
{
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new AzureAutomationClient(uri, credential, options),
"{\"value\":[]}");
var functions = new AzureAutomationFunctions(TestHelpers.CreateNullLogger<AzureAutomationFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListSubscriptionsAsync);
}
DirectConnector/ConnectorFunctionExecutor.cs:24
- This file breaks the repo’s established
await/ConfigureAwaitformatting: the awaited call and.ConfigureAwait(...)are on the same line. Other functions consistently use a line break before.ConfigureAwait(...)(e.g., DirectConnector/AzureTablesFunctions.cs:43-45).
{
var value = await operation().ConfigureAwait(continueOnCapturedContext: false);
var response = request.CreateResponse(HttpStatusCode.OK);
DirectConnector/AzureAutomationFunctions.cs:40
- To match the codebase’s multi-line async-enumeration style (see DirectConnector/AzureLogAnalyticsFunctions.cs:53-56), split the
await foreachsource so the method call starts on its own line.
await foreach (var subscription in this._client.SubscriptionsListAsync(cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false))
DirectConnector/AzureVMFunctions.cs:40
- To match the codebase’s multi-line async-enumeration style (see DirectConnector/AzureLogAnalyticsFunctions.cs:53-56), split the
await foreachsource so the method call starts on its own line.
await foreach (var subscription in this._client.SubscriptionsListAsync(cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false))
DirectConnector/AzureDigitalTwinsFunctions.cs:40
- To match the codebase’s multi-line async-enumeration style (see DirectConnector/AzureLogAnalyticsFunctions.cs:53-56), split the
await foreachsource so the method call starts on its own line.
await foreach (var model in this._client.ListModelsAsync(cancellationToken: cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false))
DirectConnector.Tests/AzureQueuesFunctionsTests.cs:176
- This test is meant to validate the SDK 0.14
NextVisibleTimeprojection, but it only asserts that the JSON contains the property name. IfNextVisibleTimedeserializes as null, the test would still pass. Parse the response JSON and assert thenextVisibleTimevalue is not null.
Assert.AreEqual(HttpStatusCode.OK, response.StatusCode);
var body = ((MockHttpResponseData)response).GetBodyAsString();
Assert.IsTrue(body.Contains("SDK 0.14 validation", StringComparison.Ordinal));
Assert.IsTrue(body.Contains("nextVisibleTime", StringComparison.Ordinal));
}
DirectConnector/TeamsFunctions.cs:140
- For consistency with the other operation lambda in this file, pass the cancellation token as a named argument. This avoids accidentally binding to the wrong overload if the generated client signature changes.
request,
this._logger,
operationName: "GetTeam",
operation: () => this._teamsClient.GetTeamAsync(teamId, cancellationToken),
cancellationToken)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (11)
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:43
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new AzureDigitalTwinsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new AzureDigitalTwinsFunctions(TestHelpers.CreateNullLogger<AzureDigitalTwinsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListModelsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:53
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new AzureVMClient(uri, credential, options),
"{\"value\":[]}");
var functions = new AzureVMFunctions(TestHelpers.CreateNullLogger<AzureVMFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListSubscriptionsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:63
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new KeyVaultClient(uri, credential, options),
"{\"value\":[]}");
var functions = new KeyVaultFunctions(TestHelpers.CreateNullLogger<KeyVaultFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListSecretsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:73
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new MicrosoftBookingsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new MicrosoftBookingsFunctions(TestHelpers.CreateNullLogger<MicrosoftBookingsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListBusinessesAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:83
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new Office365GroupsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new Office365GroupsFunctions(TestHelpers.CreateNullLogger<Office365GroupsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListGroupsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:93
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new Office365GroupsMailClient(uri, credential, options),
"{\"value\":[]}");
var functions = new Office365GroupsMailFunctions(TestHelpers.CreateNullLogger<Office365GroupsMailFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListGroupsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:103
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new OnenoteClient(uri, credential, options),
"[]");
var functions = new OnenoteFunctions(TestHelpers.CreateNullLogger<OnenoteFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListNotebooksAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:113
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new PowerBIClient(uri, credential, options),
"{\"value\":[]}");
var functions = new PowerBIFunctions(TestHelpers.CreateNullLogger<PowerBIFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListWorkspacesAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:123
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new ShiftsClient(uri, credential, options),
"{\"value\":[]}");
var functions = new ShiftsFunctions(TestHelpers.CreateNullLogger<ShiftsFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListTeamsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:133
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new TodoClient(uri, credential, options),
"[]");
var functions = new TodoFunctions(TestHelpers.CreateNullLogger<TodoFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListTaskListsAsync);
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:34
- This test returns the Task from AssertSuccessAsync while the mocked connector client is wrapped in
using var. Theusingscope ends as soon as the test method returns, so the client can be disposed before the async assertion runs, causing flaky failures. Make the testasync Taskandawaitthe assertion before leaving theusingscope.
This issue also appears in the following locations of the same file:
- line 39
- line 49
- line 59
- line 69
- line 79
- ...and 5 more
using var client = AdditionalConnectorFunctionsTests.CreateClient(
(uri, credential, options) => new AzureAutomationClient(uri, credential, options),
"{\"value\":[]}");
var functions = new AzureAutomationFunctions(TestHelpers.CreateNullLogger<AzureAutomationFunctions>(), client);
return AdditionalConnectorFunctionsTests.AssertSuccessAsync(functions.ListSubscriptionsAsync);
Co-authored-by: Dobby <dobby@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (4)
DirectConnector/PlannerFunctions.cs:94
- PlannerListMyTasksAsync handles ConnectorException, but any other non-fatal exception (e.g., serialization failures) will currently bubble out and return an unstructured 500. Catching non-fatal exceptions here keeps error responses consistent with other samples.
catch (ConnectorException ex)
{
this._logger.LogError(ex, "PlannerListMyTasks failed with status '{StatusCode}'.", ex.Status);
var response = request.CreateResponse(HttpStatusCode.BadGateway);
DirectConnector/AzureAutomationFunctions.cs:124
- CreateBadRequestAsync is used by both GetJobStatusAsync (needs jobId) and CreateJobAsync (needs runbookName), but the returned error message only mentions 'jobId', which is misleading for the create-job route.
var response = request.CreateResponse(System.Net.HttpStatusCode.BadRequest);
await response
.WriteAsJsonAsync(
new { success = false, error = "Query parameters 'subscriptionId', 'resourceGroup', 'automationAccount', and 'jobId' are required." },
cancellationToken)
DirectConnector/PlannerFunctions.cs:62
- PlannerListGroupsAsync handles ConnectorException, but any other non-fatal exception will currently bubble out of the function and return an unstructured 500. Other functions in this repo typically catch non-fatal exceptions and return a consistent JSON error response.
This issue also appears on line 90 of the same file.
catch (ConnectorException ex)
{
this._logger.LogError(ex, "PlannerListGroups failed with status '{StatusCode}'.", ex.Status);
var response = request.CreateResponse(HttpStatusCode.BadGateway);
DirectConnector/AzureQueuesFunctions.cs:191
- AzureQueuesGetMessagesAsync catches ConnectorException but not other non-fatal exceptions. For consistency with the other AzureQueuesFunctions endpoints (and to avoid unhandled exceptions returning non-JSON 500s), add the same non-fatal exception handling here.
catch (ConnectorException ex)
{
this._logger.LogError(ex, "AzureQueuesGetMessages failed with status '{StatusCode}'.", ex.Status);
var response = request.CreateResponse(HttpStatusCode.BadGateway);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (14)
DirectConnector/PlannerFunctions.cs:90
- Only connector failures are normalized here; non-fatal failures from asynchronous paging or response serialization escape without the structured 500 response used by the shared executor (
ConnectorFunctionExecutor.cs:42-51). Add the standard non-fatal exception path.
catch (ConnectorException ex)
DirectConnector/AzureAutomationFunctions.cs:15
- This summary is inaccurate because
CreateJobAsyncbelow starts a runbook job, which is a mutating operation rather than read-only discovery. Update the class documentation so consumers do not assume every endpoint is side-effect free.
/// Azure Functions demonstrating read-only Azure Automation discovery.
DirectConnector/AzureAutomationFunctions.cs:124
- This shared response is also used by
CreateJobAsync, but that endpoint requiresrunbookName, notjobId. A request missingrunbookNametherefore receives incorrect remediation guidance; pass an operation-specific message or use a message valid for both callers.
new { success = false, error = "Query parameters 'subscriptionId', 'resourceGroup', 'automationAccount', and 'jobId' are required." },
DirectConnector.Tests/AzureQueuesFunctionsTests.cs:175
- Checking only the property name still passes when deserialization leaves
NextVisibleTimenull, so this does not verify the SDK 0.14 rename that the test targets. Assert that the projected property is non-null (or equals the expected timestamp).
Assert.IsTrue(body.Contains("nextVisibleTime", StringComparison.Ordinal));
DirectConnector/PlannerFunctions.cs:62
- Only connector failures are normalized here; non-fatal failures from enumeration or response serialization escape the function without the structured 500 response used by the shared executor (
ConnectorFunctionExecutor.cs:42-51). Add the standard non-fatal exception path for consistent endpoint behavior.
This issue also appears on line 90 of the same file.
catch (ConnectorException ex)
DirectConnector/AzureQueuesFunctions.cs:191
- This new endpoint omits the non-fatal exception handling used by both existing methods in this class (
AzureQueuesFunctions.cs:65-75and125-135). Deserialization or response-writing failures will therefore bypass the endpoint's structured error contract; add the same 500 path here.
catch (ConnectorException ex)
DirectConnector/ShiftsFunctions.cs:52
- The page size is an unexplained magic number. Introduce a named constant (for example,
DefaultPageSize) so the sample documents the intent and has one place to tune it.
pageSize: 20,
DirectConnector/Office365GroupsFunctions.cs:47
- This loop unconditionally stops after the first page, so groups beyond the first 20 are silently inaccessible and no continuation token is returned. Iterate all pages (as the Planner sample does) or expose continuation-token input/output.
break;
DirectConnector/Office365GroupsMailFunctions.cs:65
- This endpoint silently discards every conversation page after the first and does not return a continuation token, so groups with more than 20 conversations cannot be fully listed. Iterate all pages or expose continuation-token input/output.
break;
DirectConnector/ConnectorFunctionExecutor.cs:23
- The awaited call keeps
ConfigureAwaiton the invocation line, unlike the multi-line chaining used immediately below atConnectorFunctionExecutor.cs:25-27. Split the continuation configuration onto the chained line.
var value = await operation().ConfigureAwait(continueOnCapturedContext: false);
DirectConnector/AzureAutomationFunctions.cs:41
- The async member invocation remains on the same line as
this._client, unlike the established chaining atOffice365GroupsFunctions.cs:39-43. StartSubscriptionsListAsyncon the chained line.
await foreach (var subscription in this._client.SubscriptionsListAsync(cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false))
DirectConnector/AzureVMFunctions.cs:40
- The async member invocation remains on the same line as
this._client, unlike the established chaining atOffice365GroupsFunctions.cs:39-43. StartSubscriptionsListAsyncon the chained line.
await foreach (var subscription in this._client.SubscriptionsListAsync(cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false))
DirectConnector.Tests/AdditionalConnectorFunctionsTests.cs:248
- The awaited operation is invoked on the same line as
await, unlike the multi-line pattern already used atAdditionalConnectorFunctionsTests.cs:222-224. Move the invocation arguments onto chained lines beforeConfigureAwait.
var response = await operation(TestHelpers.CreateRequest(), CancellationToken.None)
.ConfigureAwait(continueOnCapturedContext: false);
DirectConnector/AzureDigitalTwinsFunctions.cs:40
- The async member invocation remains on the same line as
this._client, unlike the established chaining atOffice365GroupsFunctions.cs:39-43. StartListModelsAsyncon the chained line.
await foreach (var model in this._client.ListModelsAsync(cancellationToken: cancellationToken)
.ConfigureAwait(continueOnCapturedContext: false))
PR 72 Review FindingsPR: Azure/Connectors-NET-Samples#72 Reviewed head: Recommended disposition: Request changes Review Body[Dobby] Code review: REQUEST_CHANGESI found two issues on head Inline Comment 1File: Place on the error message in
Automation ResolutionMake the bad-request response endpoint-specific:
Inline Comment 2File: Place on the
Azure Queues ResolutionReplace the property-name check with a value assertion. For example: using var document = JsonDocument.Parse(body);
var nextVisibleTime = document.RootElement
.GetProperty("messages")[0]
.GetProperty("nextVisibleTime")
.GetString();
Assert.AreEqual("2026-08-19T03:00:00Z", nextVisibleTime);Steps For The PR Owner
Validation Context
|
Co-authored-by: Dobby <dobby@microsoft.com>
|
[Dobby] Addressed the requested changes from the review findings in commit 4d7e9b7.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
DirectConnector/Office365GroupsFunctions.cs:47
- The SDK method returns items across all pages, but this unconditional
breaksilently limits the endpoint to the first page (20 groups). Because the response exposes no continuation token, later groups are unreachable; enumerate all pages or return paging state.
{
groups.AddRange(page.Values);
break;
DirectConnector/Office365GroupsMailFunctions.cs:65
- The SDK method returns conversations across all pages, but this unconditional
breakdiscards every continuation after the first page. Since callers receive no continuation token, conversations beyond the first page cannot be retrieved; enumerate all pages or expose paging state.
{
conversations.AddRange(page.Values);
break;
DirectConnector/AzureAutomationFunctions.cs:15
- This class is not read-only:
CreateJobAsyncstarts a runbook job. Update the summary so consumers are not misled about the sample's side effects.
/// Azure Functions demonstrating read-only Azure Automation discovery.
|
Findings P1: paged results are silently truncated. Both group and conversation endpoints stop after the first requested 20-item page without returning a continuation token. Later records are unreachable. Remove break to auto-page, or expose continuation state. Office365GroupsFunctions.cs#L39-L47, Office365GroupsMailFunctions.cs#L56-L65 P2: tests dispose connector clients before returned tasks complete. The success tests use using var client and return an unawaited task, causing disposal when the test method returns. Make them async Task and await AssertSuccessAsync. AdditionalConnectorFunctionsTests.cs#L28-L34 P2: Azure Automation is incorrectly described as read-only. The class exposes CreateJobAsync, which starts a runbook job. This documentation could mislead users about side effects. AzureAutomationFunctions.cs#L14-L17 |
Co-authored-by: Dobby <dobby@microsoft.com>
|
[Dobby] Addressed all findings from this review in commit a40b6d7.
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Suppressed comments (1)
DirectConnector/AzureQueuesFunctions.cs:174
GetMessagesAsynchides each returned message, but this projection dropsPopReceipt, which the SDK model identifies as the token required to delete that message. Return it so callers can complete the receive/delete workflow rather than having every message reappear after its visibility timeout.
message.MessageId,
message.MessageText,
message.DequeueCount,
nextVisibleTime = message.NextVisibleTime,
});
Co-authored-by: Dobby <dobby@microsoft.com>
Co-authored-by: Dobby <dobby@microsoft.com>
Swapnil Nagar (swapnil-nagar)
left a comment
There was a problem hiding this comment.
[P2] Handle non-connector failures in Azure Queues
AzureQueuesFunctions.cs:183
AzureQueuesGetMessagesAsync only catches ConnectorException. Malformed successful JSON raises JsonException, escaping the structured { success: false } contract. Add the same non-fatal fallback used by adjacent handlers or use ConnectorFunctionExecutor.
[P2] Handle non-connector failures in Planner endpoints
PlannerFunctions.cs:52 and PlannerFunctions.cs:90
Both handlers have the same gap. Deserialization or local serialization failures become unstructured host errors instead of the sample’s standard JSON 500 response.
Summary
Azure.Connectors.Sdkfrom 0.13.0-preview.1 to 0.14.0-preview.1.GetTeamAsyncandListTeamMembersAsync.Messages.QueueMessagesList.QueueMessageand the renamedNextVisibleTimeC# property.SDK 0.14 contract coverage
AsyncPageable<GetTaskResponseV2>, covering the 0.14 versioned type rename.ListGroupsAsyncandListConversationsAsyncso sample responses are not truncated.NextVisibleTimeand thePopReceiptrequired to delete each message.GetAllTeamsAsync.CreateJobAsyncand reads it throughGetStatusOfJobAsync.VirtualMachineGetAsync,ListConversationsAsync,GetScorecardsAsync, andListShiftsCrossTeamAsync.Validation
dotnet test Connectors-NET-Samples.sln --configuration Release: 76 passed, 0 failed.dotnet format Connectors-NET-Samples.sln --verify-no-changes --no-restore: passed.0eba74f59c844a83a156440f6940c5b6.sdk-test-gateway-prodConnector Namespace.nextVisibleTime, and verified cleanup left no temporary queue.Follow-up findings
x-ms-api-version=2017-09-01-preview, which Azure rejects withInvalidApiVersionParameter. Tracked by Azure/Connectors-NET-SDK#256.boxmcpserveris registered but has no generated typed client or DI extension. Tracked by Azure/Connectors-NET-SDK#257.AddCloudconvertClientDI extension. Tracked by Azure/Connectors-NET-SDK#258.Trigger note
dataverse-developer-onnewitemspolls and reportsSkipped/Acceptedwhen no matching row is available.dataverse-diagnostic-onnewitemsreaches Dataverse but receives HTTP 403 because its authenticated Dataverse user lacksprvReadAccount; this is an environment permission issue rather than an SDK regression.Authored with Dobby.