Spring Cloud Azure App Configuration 4.0 #33337

mrm9084 · 2023-02-06T20:04:43Z

Description

This updates the App Configuration Spring Provider to the 4.0 format, adds support for global properties

All SDK Contribution checklist:

The pull request does not introduce [breaking changes]
CHANGELOG is updated for new features, bug fixes or other significant changes.
I have read the contribution guidelines.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

saragluna · 2023-02-07T09:13:02Z

eng/code-quality-reports/src/main/resources/revapi/revapi.json

+        {
+          "code": "java.class.externalClassExposedInAPI",
+          "new": "class com.azure.data.appconfiguration.models.SettingSelector",
+          "justification": "This is an Azure SDK class that is used in the public API."
+        },


Can we use one rule as following for them?

{ "regex": true, "code": "java\\.class\\.externalClassExposedInAPI", "new": "(interface|class|enum) com\\.azure\\.(communication\\.common|core|cosmos|data\\.schemaregistry|json|messaging\\.eventgrid|messaging\\.eventhubs|messaging\\.servicebus|resourcemanager|security\\.keyvault|storage).*", "justification": "SDK classes are allowed to be exposed by dependencies using them." },

Or simply modify that rule?

we should avoid regex as it increases the revapi time. See Alan's post about it.

I don't think this mean we should avoid regex,

When adding many suppressions determine if they can be compacted with existing suppressions for that kind

RevApi has regex suppressions

Checkstyle uses

Spotbugs has regex suppressions

But I checked the PR, he is using code to address the external APIs. See eng/code-quality-reports/src/main/java/com/azure/tools/revapi/transforms/AzureSdkAllowedExternalApis.java in #33130.

We can wait until #33130 gets merged.

eng/versioning/version_client.txt

saragluna · 2023-02-07T09:20:38Z

sdk/appconfiguration/tests.yml

@@ -4,16 +4,12 @@ stages:
  - template: /eng/pipelines/templates/stages/archetype-sdk-tests.yml


Can we delete this file now?

Doesn't the SDK also use that file?

Not sure, check this out https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2169438&view=logs&j=1bcb7d81-906d-5a53-3f29-b3b6382abd7c&t=26e36aca-4901-52a9-d557-4eb0b6782169&l=15.

This should be kept and the Artifacts that were removed should instead be replaced with

Artifacts: - name: azure-data-appconfiguration groupId: com.azure safeName: azuredataappconfiguration

sdk/spring/spring-cloud-azure-appconfiguration-config-web/pom.xml

saragluna · 2023-02-07T10:05:48Z

.../azure/spring/cloud/config/web/implementation/pullrefresh/AppConfigurationEventListener.java

@@ -1,10 +1,10 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
-package com.azure.spring.cloud.config.web.pullrefresh;
+package com.azure.spring.cloud.config.web.implementation.pullrefresh;


do you think refresh.pull and refresh.push would be better as the package names?

...ppconfiguration-config-web/src/main/java/com/azure/spring/cloud/config/web/package-info.java

sdk/spring/spring-cloud-azure-appconfiguration-config/pom.xml

saragluna · 2023-02-07T10:09:50Z

...configuration-config/src/main/java/com/azure/spring/cloud/config/KeyVaultSecretProvider.java

@@ -8,10 +8,10 @@
 public interface KeyVaultSecretProvider {


We still need this?

This is for resolving key vault references without connecting to key vault. It has no "client", it is instead of.

But internally we will build the KV clients?

So the way this works is before we check if a client even exists we pass the KV Reference to the KeyVaultSecretProvider, if it returns any non-Null value than that is returned as the secret. If nothing is returned or this isn't configured only then do we create a client or check if one has already been made.

...ity-reports/src/main/java/com/azure/tools/revapi/transforms/AzureSdkAllowedExternalApis.java

sdk/spring/spring-cloud-azure-actuator/pom.xml

saragluna · 2023-02-14T06:57:22Z

.../com/azure/spring/cloud/actuator/appconfiguration/AppConfigurationConfigHealthIndicator.java

+/**
+ * Indicator class of App Configuration 
+ */
+public final class AppConfigurationConfigHealthIndicator implements HealthIndicator {


Let's put this into the impl package. I know the other classes were exposed as public API, but we have changed their visibility in Spring Boot 3 branch. So for the new classes added to the actuator module, let's keep them in the impl package.

I don't think we can as then spring-cloud-azure-actuator-autoconfigure would be trying to access a private class of another package.

How about moving it to the implementation package then?

I thought we didn't access implementation code of other packages?

...cloud/actuator/autoconfigure/appconfiguration/AppConfigurationConfigHealthConfiguration.java

sdk/appconfiguration/tests.yml

mrm9084 · 2023-02-15T08:38:10Z

/azp run java - spring - tests

azure-pipelines · 2023-02-15T08:38:24Z

Azure Pipelines successfully started running 1 pipeline(s).

saragluna · 2023-02-15T15:56:15Z

/azp run java - spring - tests

azure-pipelines · 2023-02-15T15:56:29Z

Azure Pipelines successfully started running 1 pipeline(s).

mrm9084 · 2023-02-15T17:45:35Z

/azp run java - spring - tests

azure-pipelines · 2023-02-15T17:45:49Z

Azure Pipelines successfully started running 1 pipeline(s).

mrm9084 · 2023-02-15T18:52:44Z

/azp run java - spring - tests

azure-pipelines · 2023-02-15T18:53:00Z

Azure Pipelines successfully started running 1 pipeline(s).

Moving Spring Provider to the Spring folder

84f5744

mrm9084 requested review from backwind1233, chenrujun, hui1110, Netyyyy, saragluna, stliu, yiliuTo, moarychan, fangjian0423, mssfang and alzimmermsft as code owners February 6, 2023 20:04

ghost added App Configuration Azure.ApplicationModel.Configuration azure-spring All azure-spring related issues labels Feb 6, 2023

Fixing name in external places and updating RevApi

cebd01c

mrm9084 requested review from samvaity, g2vinay, JimSuplizio, JonathanGiles, hallipr, weshaggard and benbp as code owners February 6, 2023 20:43

mrm9084 added 6 commits February 6, 2023 12:51

Removing Dynamic Features as it might not be part of the 4.0

a8751c9

Fixing Dependencies, and Dynamic Web

c612103

Updating Readmes and dependencies

c7bbe41

Fixing Links

92df7f4

Updated Test Project

37ecab2

Fixing names

3da321c

saragluna requested changes Feb 7, 2023

View reviewed changes

Code Review Comments + Fix to Feature FIlters

2f4e17a

mrm9084 added 10 commits February 13, 2023 11:00

Fixing Auto Configuration and revapi

a7ef855

Merge branch 'main' into 4.0Merge

187e963

Updating RevApi

cfa2b82

Fixing AutoConfiguration

a906752

Fixing Verify

5fd7627

Wrong dependency

4ac3db2

Update pom.xml

1c6a5fe

Update pom.xml

a7a40eb

Update pom.xml

61a9854

Update pom.xml

d0f9453

saragluna reviewed Feb 14, 2023

View reviewed changes

...ity-reports/src/main/java/com/azure/tools/revapi/transforms/AzureSdkAllowedExternalApis.java Show resolved Hide resolved

saragluna requested changes Feb 14, 2023

View reviewed changes

alzimmermsft reviewed Feb 14, 2023

View reviewed changes

sdk/appconfiguration/tests.yml Outdated Show resolved Hide resolved

mrm9084 added 2 commits February 14, 2023 23:57

Moving files to impl, and adding optional

1dfe5cd

Merge branch 'main' into 4.0Merge

37d14b5

saragluna approved these changes Feb 15, 2023

View reviewed changes

Update tests.yml

b576995

move app config to the first when provisioning resources

3b55ca3

Fixing Integration Test

71ba16f

Fixing Integration Tests

7e6488c

mrm9084 merged commit 72884d4 into Azure:main Feb 15, 2023

mrm9084 deleted the 4.0Merge branch April 11, 2023 21:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spring Cloud Azure App Configuration 4.0 #33337

Spring Cloud Azure App Configuration 4.0 #33337

mrm9084 commented Feb 6, 2023

saragluna Feb 7, 2023

mrm9084 Feb 7, 2023

saragluna Feb 8, 2023 •

edited

Loading

saragluna Feb 10, 2023

saragluna Feb 7, 2023

mrm9084 Feb 7, 2023

saragluna Feb 8, 2023

alzimmermsft Feb 14, 2023 •

edited

Loading

saragluna Feb 7, 2023

saragluna Feb 7, 2023

mrm9084 Feb 7, 2023

saragluna Feb 8, 2023

mrm9084 Feb 8, 2023

saragluna Feb 14, 2023

mrm9084 Feb 14, 2023

saragluna Feb 15, 2023 •

edited

Loading

mrm9084 Feb 15, 2023

mrm9084 commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

saragluna commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

mrm9084 commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

mrm9084 commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

		@@ -4,16 +4,12 @@ stages:
		- template: /eng/pipelines/templates/stages/archetype-sdk-tests.yml

Spring Cloud Azure App Configuration 4.0 #33337

Spring Cloud Azure App Configuration 4.0 #33337

Conversation

mrm9084 commented Feb 6, 2023

Description

All SDK Contribution checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saragluna Feb 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alzimmermsft Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saragluna Feb 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrm9084 commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

saragluna commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

mrm9084 commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

mrm9084 commented Feb 15, 2023

azure-pipelines bot commented Feb 15, 2023

saragluna Feb 8, 2023 •

edited

Loading

alzimmermsft Feb 14, 2023 •

edited

Loading

saragluna Feb 15, 2023 •

edited

Loading