Skip to content

Azure App Configuration client integration #8945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 22 commits into from
May 12, 2025
Merged

Azure App Configuration client integration #8945

merged 22 commits into from
May 12, 2025

Conversation

zhiyuanliang-ms
Copy link
Contributor

@zhiyuanliang-ms zhiyuanliang-ms commented Apr 24, 2025 •
edited by IEvangelist
Loading

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #787

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@github-actions github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label Apr 24, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 24, 2025
@zhiyuanliang-ms zhiyuanliang-ms mentioned this pull request Apr 24, 2025
Closed
10 tasks
zhiyuanliang-ms
zhiyuanliang-ms commented Apr 24, 2025
View reviewed changes
Directory.Packages.props Outdated
@@ -23,6 +23,7 @@
<PackageVersion Include="Azure.Security.KeyVault.Secrets" Version="4.7.0" />
<PackageVersion Include="Azure.Storage.Blobs" Version="12.24.0" />
<PackageVersion Include="Azure.Storage.Queues" Version="12.22.0" />
<PackageVersion Include="Microsoft.Extensions.Configuration.AzureAppConfiguration" Version="8.1.1" />
Copy link
Contributor Author

@zhiyuanliang-ms zhiyuanliang-ms Apr 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Waiting for 8.2.0 to have tracing support. (Hopefully, it will be released next week)

Copy link
Member

Choose a reason for hiding this comment

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

Would the current preview version have the tracing feature? If so then you could complete the PR and just switch the version when it's no more preview.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the current preview version doesn't support tracing.

@zhiyuanliang-ms
Copy link
Contributor Author

@jimmyca15 @zhenlan @drago-draganov Could you help review this PR?

sebastienros
sebastienros reviewed Apr 24, 2025
View reviewed changes
@sebastienros sebastienros changed the title Aspire component for Azure App Configuration Azure App Configuration client integration Apr 24, 2025
zhenlan
zhenlan reviewed May 1, 2025
View reviewed changes
sebastienros
sebastienros reviewed May 6, 2025
View reviewed changes
Copy link
Member

@sebastienros sebastienros left a comment

Choose a reason for hiding this comment

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

Please remove the DisableTracing references until the package actually supports it, otherwise we would ship something that doesn't have any tracing but still the documentation and the code that states so.

Other option is we wait for the package to merge this PR.

zhenlan
zhenlan reviewed May 6, 2025
View reviewed changes
@zhenlan
Copy link
Member

zhenlan commented May 6, 2025

Please remove the DisableTracing references until the package actually supports it, otherwise we would ship something that doesn't have any tracing but still the documentation and the code that states so.

Other option is we wait for the package to merge this PR.

Agreed. We should drop everything about tracing or health check, so this PR can be merged. We can add them in new PRs when those features are ready.

@zhiyuanliang-ms zhiyuanliang-ms force-pushed the zhiyuanliang/app-config-provider-component branch from e0186a6 to fc6a0dc Compare May 7, 2025 05:14
@zhiyuanliang-ms zhiyuanliang-ms force-pushed the zhiyuanliang/app-config-provider-component branch from fc6a0dc to 74b7956 Compare May 7, 2025 05:16
@zhiyuanliang-ms
Copy link
Contributor Author

@zhenlan @sebastienros everything about tracing is removed from this PR.

zhenlan
zhenlan reviewed May 7, 2025
View reviewed changes
eerhardt
eerhardt reviewed May 7, 2025
View reviewed changes
sebastienros
sebastienros reviewed May 12, 2025
View reviewed changes
@sebastienros
Copy link
Member

The project needs to be renamed, check discussion

@joperezr joperezr added this to the 9.3 milestone May 12, 2025
@sebastienros sebastienros merged commit 39b434f into dotnet:main May 12, 2025
174 checks passed
@sebastienros sebastienros mentioned this pull request May 12, 2025
Closed
@zhiyuanliang-ms
Copy link
Contributor Author

@sebastienros @eerhardt Thank you for your help!

@davidfowl davidfowl mentioned this pull request May 13, 2025
Merged
@zhiyuanliang-ms zhiyuanliang-ms mentioned this pull request May 19, 2025
Open
@zhiyuanliang-ms zhiyuanliang-ms deleted the zhiyuanliang/app-config-provider-component branch June 2, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidfowl davidfowl

@DamianEdwards DamianEdwards

@RussKie RussKie

@zhenlan zhenlan

@sebastienros sebastienros

@eerhardt eerhardt

@radical radical

Assignees
No one assigned
Labels
area-integrations Issues pertaining to Aspire Integrations packages community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
9.3
Development

Successfully merging this pull request may close these issues.

Add Azure App Config Aspire Client Integration
8 participants
@zhiyuanliang-ms @zhenlan @sebastienros @davidfowl @DamianEdwards @RussKie @eerhardt @joperezr