Merge main to release/stable/v8 #652
Conversation
…eature is enabled. (#633) * Ensure that kv collection refresh interval is not used unless collection based refresh of key-values is enabled. Add tests to ensure that minimum refresh interval is respected for key-values and feature flags. * Remove duplicated tests. * fix. * Fix formatting.
Co-authored-by: AMER JUSUPOVIC <ajusupovic@microsoft.com>
* use TryAddSingleton instead of AddSingleton in extension, fix refresherprovider accordingly * remove unused using * add comment
* fix format * fix format * fix format * fix format
* add initial content type pattern * format fix * fix comment * update to account for chat completion vs ai profiles * in progress fix adapter to use existing requesttracingoptions * use content type tracing to pass to requesttracingoptions * fix comments and naming * remove unneeded file * add check for request tracing enabled * check content type in preparedata * remove errors * fix spacing * fix test * remove unused usings, add back catch for .net framework * fix parsing * rename constants * fix indent * update for PR comments * PR comments, update if conditions * add isjson check * update isjson extension
* fix test to remove obsolete * change variable names
* fix isjson and separate exclusion logic * update adapters to use new extension * PR comments
There was a problem hiding this comment.
Pull Request Overview
This PR merges updates from main into the release/stable/v8 branch for version 8.1.2, incorporating asynchronous improvements in tests, new request tracing capabilities for AI configuration, and various enhancements to Azure App Configuration handling.
- Replaces blocking Thread.Sleep calls with asynchronous Task.Delay and updates test signatures.
- Introduces new content type extension methods and AI configuration tracking in request tracing.
- Refines refresh logic and HTTP transport configuration in core classes.
Reviewed Changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Tests.AzureAppConfiguration/TestHelper.cs | Added delay parameter and async update in MockAsyncPageable. |
| tests/Tests.AzureAppConfiguration/RefreshTests.cs | Replaced Thread.Sleep with Task.Delay and updated async test signatures. |
| tests/Tests.AzureAppConfiguration/KeyVaultReferenceTests.cs | Fixed lambda syntax by removing stray semicolons. |
| tests/Tests.AzureAppConfiguration/FeatureManagementTests.cs | Marked tests as obsolete and updated variable naming for clarity. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/RequestTracingOptions.cs | Added AI configuration tracing flags and corresponding update/reset methods. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/JsonKeyValueAdapter.cs | Updated CanProcess logic to use TryParseContentType and content type extension methods. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/IConfigurationSettingPageIterator.cs | Adjusted using order to align with updated project standards. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/FeatureManagement/FeatureManagementKeyValueAdapter.cs | Improved null and whitespace checks in the CanProcess method. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/StringExtensions.cs | Added TryParseContentType extension method for robust content type parsing. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Extensions/ContentTypeExtensions.cs | Introduced helper methods for checking JSON, AI, feature flag, and KeyVault content types. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Constants/RequestTracingConstants.cs | Added new constants for AI and AI Chat Completion tracing. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/ConfigurationSettingPageExtensions.cs | Updated using directives for consistency. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureKeyVaultReference/AzureKeyVaultKeyValueAdapter.cs | Improved validation in CanProcess by checking for null/whitespace values. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationRefresherProvider.cs | Updated refresher discovery logic and property getter to ensure up-to-date refreshers. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs | Refined refresh logic, added validation for refresh intervals, and updated request tracing handling. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs | Configured HTTP transport with a dedicated network timeout via HttpClientTransport. |
| src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationExtensions.cs | Updated dependency injection registration to use TryAddSingleton. |
Files not reviewed (3)
- src/Microsoft.Azure.AppConfiguration.AspNetCore/Microsoft.Azure.AppConfiguration.AspNetCore.csproj: Language not supported
- src/Microsoft.Azure.AppConfiguration.Functions.Worker/Microsoft.Azure.AppConfiguration.Functions.Worker.csproj: Language not supported
- src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Microsoft.Extensions.Configuration.AzureAppConfiguration.csproj: Language not supported
| clientOptions.Retry.MaxDelay = MaxRetryDelay; | ||
| clientOptions.Retry.Mode = RetryMode.Exponential; | ||
| clientOptions.AddPolicy(new UserAgentHeaderPolicy(), HttpPipelinePosition.PerCall); | ||
| clientOptions.Transport = new HttpClientTransport(new HttpClient() |
There was a problem hiding this comment.
Creating a new HttpClient instance inline for HttpClientTransport may lead to resource exhaustion; consider reusing an existing HttpClient or using IHttpClientFactory to manage its lifecycle.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Avoid resolving comments without a reason. This is a completely valid concern.
There was a problem hiding this comment.
The PR that added this must be reverted
| } | ||
| } | ||
| } | ||
| // Copyright (c) Microsoft Corporation. |
There was a problem hiding this comment.
Why is every single line of this file showing up oin the diff?
There was a problem hiding this comment.
Discussed offline, line endings were inconsistent for this file when merged into main from another PR
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Merging in changes for version 8.1.2