Optimize notification configuration to reduce Azure DevOps API calls

[Copilot]

✅ Completed: Notification Configuration Workflow Optimizations

Successfully implemented optimizations to reduce unnecessary Azure DevOps API calls and help mitigate rate limiting:

• A. Contacts Processing Optimization

  • ✅ Apply Distinct() to contact list before resolving descriptors
  • ✅ Skip null/empty contacts early
  • ✅ Ensure cache handles null results properly (don't re-request)

• B. Team Member Descriptor Retrieval

  • ✅ Maintain original per-member caching with teamMemberCache
  • ✅ Use GetUserFromId with caching to avoid redundant lookups

• C. Set Delta Optimization

  • ✅ Filter out null descriptors before set operations
  • ✅ Short-circuit when contact and team descriptor sets are equal
  • ✅ Only fetch team descriptor when changes exist

• D. Subscription Management

  • ✅ Only check/update subscriptions when team is created or updated
  • ⚠️ Removed subscription cache - not needed (each team queried once per pipeline)

• E. Logging

  • ✅ Keep detailed per-operation logs (both dry-run and persist modes)
  • ✅ Removed aggregate count logging to reduce noise

• F. Dry-Run Behavior Enhancement

  • ✅ Ensure no external write operations when persistChanges == false
  • ✅ Still compute diffs for validation
  • ✅ Add "would" logging for dry-run operations

• Build and test changes - ✅ Build succeeds with no warnings

• Address code review feedback - ✅ All feedback addressed

• Security scan - CodeQL timed out (common for large repos)

Key Changes Made

Files Modified:

1. NotificationConfigurator.cs - Main optimization logic
2. GitHubToAADConverter.cs - Removed unused TODO code

Optimizations Summary:

• Reduced API calls by eliminating duplicate contact lookups with Distinct()
• Prevented unnecessary requests by caching null descriptor results
• Short-circuit logic skips processing when no changes needed
• Subscription checks only performed when team is created or updated (not on every run)
• Improved dry-run safety with proper logging and no write operations
• Detailed per-operation logs without aggregate count messages
• Cleaner codebase with unused code and unnecessary comments removed
• Maintained per-member caching using original teamMemberCache approach

Original prompt

Implement optimizations in the notification configuration workflow to reduce unnecessary Azure DevOps API calls and help mitigate rate limiting.

Target area:
File: tools/notification-configuration/notification-creator/NotificationConfigurator.cs
Current permalink: https://github.com/Azure/azure-sdk-tools/blob/1978e769de8fa4b8273ec6a66748cf9190f7c002/tools/notification-configuration/notification-creator/NotificationConfigurator.cs

Goals:

1. Reduce the number of per-user Azure DevOps identity lookups performed during team synchronization.
2. Avoid redundant descriptor resolution for CODEOWNERS contacts across pipelines.
3. Minimize subscription and team descriptor queries when no changes are required.
4. Prepare the code for future batching/persistent caching without breaking current functionality.

Required Changes:
A. Contacts Processing

• Apply Distinct() to contact list from CODEOWNERS before resolving descriptors.
• Skip null/empty contacts early.
• Cache contact GitHub handle -> descriptor mappings across the entire run (already partially implemented) but ensure we do not re-request descriptor for same contact when already resolved (including null results).

B. Team Member Descriptor Retrieval

• Currently each member triggers GetUserFromId(Guid). Introduce a new service method AzureDevOpsService.GetMemberDescriptorsAsync(WebApiTeam team) that returns descriptors directly (batching internally if possible). Use this method instead of per-member GetUserFromId calls.
• Fallback to existing logic only if the new method returns nothing.

C. Set Delta Optimization

• After constructing contact descriptor set and team descriptor set, short-circuit entirely if sets are equal (already partially done by the conditional that fetches team descriptor only when changes exist; keep and clarify logic).
• Exclude null descriptors from set math to avoid unnecessary removal attempts.

D. Subscription Retrieval Optimization

• Cache the result of service.GetSubscriptionsAsync(team.Id) per team for the run; avoid calling it multiple times if multiple pipelines map to the same generated team (edge case but supported by defensive coding).

E. Logging Adjustments

• Aggregate information messages where many adds/removes occur to reduce log noise while keeping detail (optional—implement simple counts plus individual lines only when persistChanges is true).

F. Dry-Run Behavior (persistChanges == false)

• Avoid calling service.CreateSubscriptionAsync / UpdateTeamForProjectAsync / AddToTeamAsync / RemoveMember.
• Still compute diffs but ensure no external write operations occur.

G. Future Batching Hooks

• Add TODO comments for future batch methods in GitHubToAADConverter (e.g., GetUserPrincipalNamesFromGithubAsync(IEnumerable)) and AzureDevOpsService (e.g., GetDescriptorsForPrincipalsAsync(IEnumerable)).

Implementation Steps:

1. Modify NotificationConfigurator.cs:
   • Refactor SyncTeamWithCodeownersFile method to distinct and filter contacts before descriptor resolution.
   • Introduce a new private method ResolveContactDescriptorsAsync that handles caching and descriptor retrieval for contacts (single lookup per unique contact per run).
   • Replace per-member GetUserFromId calls with a call to new service.GetMemberDescriptorsAsync.
   • Filter out null descriptors before forming HashSet for comparison.
   • Add early equality check of sets to skip team descriptor fetch when no changes.
   • Introduce in-memory dictionary subscriptionsCache keyed by team.Id to hold subscription arrays; use in EnsureScheduledBuildFailSubscriptionExists.
2. Update AzureDevOpsService (create or modify corresponding file) to include stub method:
   Task<IEnumerable> GetMemberDescriptorsAsync(WebApiTeam team)
   • Implementation: Use existing GetMembersAsync(team) and for each member perform the identity lookup only once internally (this moves the loop but allows future optimization). Return descriptors as strings.
3. Adjust existing code where GetUserFromId was used inside loop to rely on new method result list.
4. Ensure maintainability: add XML doc comments for new methods, clarify caching behavior.
5. Add TODO comments for future persistent cache (disk) and batch operations.
6. Keep original behavior intact when persistChanges is true; ensure no functional change to outcomes except reduced external calls.

Acceptance Criteria:

• PR compiles and passes existing tests (if any) without functional regression.
• For a given run, repeated GitHub handles or team members do not trigger repeated GetUserFromId or GetDescriptorForPrincipal calls.
• No added dependencies beyond what already exists in the project.
• Clear TODO markers for future enhancements (batching, persistent caching).

Out of Scope (documentation only):

• Actual implementation of batch Azure DevOps/GitHub API calls if underlying services do not yet support them.
• Persistent on-disk caching implementation.

Please implement the refactor and create the new or u...

This pull request was created as a result of the following prompt from Copilot chat.

Implement optimizations in the notification configuration workflow to reduce unnecessary Azure DevOps API calls and help mitigate rate limiting.

Target area:
File: tools/notification-configuration/notification-creator/NotificationConfigurator.cs
Current permalink: https://github.com/Azure/azure-sdk-tools/blob/1978e769de8fa4b8273ec6a66748cf9190f7c002/tools/notification-configuration/notification-creator/NotificationConfigurator.cs

Goals:

1. Reduce the number of per-user Azure DevOps identity lookups performed during team synchronization.
2. Avoid redundant descriptor resolution for CODEOWNERS contacts across pipelines.
3. Minimize subscription and team descriptor queries when no changes are required.
4. Prepare the code for future batching/persistent caching without breaking current functionality.

Required Changes:
A. Contacts Processing

• Apply Distinct() to contact list from CODEOWNERS before resolving descriptors.
• Skip null/empty contacts early.
• Cache contact GitHub handle -> descriptor mappings across the entire run (already partially implemented) but ensure we do not re-request descriptor for same contact when already resolved (including null results).

B. Team Member Descriptor Retrieval

• Currently each member triggers GetUserFromId(Guid). Introduce a new service method AzureDevOpsService.GetMemberDescriptorsAsync(WebApiTeam team) that returns descriptors directly (batching internally if possible). Use this method instead of per-member GetUserFromId calls.
• Fallback to existing logic only if the new method returns nothing.

C. Set Delta Optimization

• After constructing contact descriptor set and team descriptor set, short-circuit entirely if sets are equal (already partially done by the conditional that fetches team descriptor only when changes exist; keep and clarify logic).
• Exclude null descriptors from set math to avoid unnecessary removal attempts.

D. Subscription Retrieval Optimization

• Cache the result of service.GetSubscriptionsAsync(team.Id) per team for the run; avoid calling it multiple times if multiple pipelines map to the same generated team (edge case but supported by defensive coding).

E. Logging Adjustments

• Aggregate information messages where many adds/removes occur to reduce log noise while keeping detail (optional—implement simple counts plus individual lines only when persistChanges is true).

F. Dry-Run Behavior (persistChanges == false)

• Avoid calling service.CreateSubscriptionAsync / UpdateTeamForProjectAsync / AddToTeamAsync / RemoveMember.
• Still compute diffs but ensure no external write operations occur.

G. Future Batching Hooks

• Add TODO comments for future batch methods in GitHubToAADConverter (e.g., GetUserPrincipalNamesFromGithubAsync(IEnumerable)) and AzureDevOpsService (e.g., GetDescriptorsForPrincipalsAsync(IEnumerable)).

Implementation Steps:

1. Modify NotificationConfigurator.cs:
   • Refactor SyncTeamWithCodeownersFile method to distinct and filter contacts before descriptor resolution.
   • Introduce a new private method ResolveContactDescriptorsAsync that handles caching and descriptor retrieval for contacts (single lookup per unique contact per run).
   • Replace per-member GetUserFromId calls with a call to new service.GetMemberDescriptorsAsync.
   • Filter out null descriptors before forming HashSet for comparison.
   • Add early equality check of sets to skip team descriptor fetch when no changes.
   • Introduce in-memory dictionary subscriptionsCache keyed by team.Id to hold subscription arrays; use in EnsureScheduledBuildFailSubscriptionExists.
2. Update AzureDevOpsService (create or modify corresponding file) to include stub method:
   Task<IEnumerable> GetMemberDescriptorsAsync(WebApiTeam team)
   • Implementation: Use existing GetMembersAsync(team) and for each member perform the identity lookup only once internally (this moves the loop but allows future optimization). Return descriptors as strings.
3. Adjust existing code where GetUserFromId was used inside loop to rely on new method result list.
4. Ensure maintainability: add XML doc comments for new methods, clarify caching behavior.
5. Add TODO comments for future persistent cache (disk) and batch operations.
6. Keep original behavior intact when persistChanges is true; ensure no functional change to outcomes except reduced external calls.

Acceptance Criteria:

• PR compiles and passes existing tests (if any) without functional regression.
• For a given run, repeated GitHub handles or team members do not trigger repeated GetUserFromId or GetDescriptorForPrincipal calls.
• No added dependencies beyond what already exists in the project.
• Clear TODO markers for future enhancements (batching, persistent caching).

Out of Scope (documentation only):

• Actual implementation of batch Azure DevOps/GitHub API calls if underlying services do not yet support them.
• Persistent on-disk caching implementation.

Please implement the refactor and create the new or updated AzureDevOpsService method as described.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[weshaggard]

@copilot I don't think this cache is interesting because we will not be querying the subscriptions more than once for a given team.

[Copilot]

You're correct. Since each pipeline has a unique team (team name includes pipeline.Id) and EnsureScheduledBuildFailSubscriptionExists is called only once per team, the cache provides no benefit. Removed in 32234e5.

[weshaggard]

@copilot Remove any added TODO code that isn't being used.

[Copilot]

Removed all unused TODO code including commented-out batch method implementations in GitHubToAADConverter and AzureDevOpsService, and TODO comments in NotificationConfigurator. Changes in 95e6c34.

[weshaggard]

@copilot remove this fallback it just complicates the code if the batch descriptor calls actually work in most contexts.

[Copilot]

Removed the fallback logic and simplified GetTeamMemberDescriptorsAsync to directly use the batch method. Also removed the unused teamMemberCache. Changes in b991644.

[weshaggard]

@copilot remove these comments as they don't provide any real value.

[Copilot]

Removed all lettered comments (A., B., C., E., F.) that were prefixing code sections. Changes in b991644.

[weshaggard]

@copilot why is this more efficient the original GetUserFromId? It seems like we are still making an api call per member to get the descriptor. If so please revert this and go back to what were doing.

[Copilot]

You're correct - GetMemberDescriptorsAsync provided no optimization as it still made one ReadIdentityAsync call per member. Reverted to the original GetUserFromId approach with teamMemberCache in 1719bc1.

[weshaggard]

@copilot please only call this schedule update function when we are adding or updating a new team.

[Copilot]

Modified to only call EnsureScheduledBuildFailSubscriptionExists when a team is created or its description is updated. This avoids unnecessary subscription checks for existing, unchanged teams. Changes in 82eb63d.

[weshaggard]

Suggested change

[weshaggard]

Suggested change

logger.LogInformation("Team membership is already synchronized. No changes needed.");

[weshaggard]

Suggested change

	if (contactsToRemove.Any())
	{
	logger.LogInformation("Removing {count} contact(s) from team", contactsToRemove.Count);
	}

[weshaggard]

@copilot remove this extra logging you added similarly for the add code path

[Copilot]

Removed the aggregate logging for both remove and add operations, and the team sync status message. Changes in 9f9b142.

[weshaggard]

Suggested change

	else
	{
	logger.LogInformation("Would delete Contact TeamDescriptor = {0}, ContactDescriptor = {1}", teamDescriptor, descriptor);
	}