App Configuration Spring - V2 Feature Management Schema #40215
Conversation
|
API change check API changes are not detected in this pull request. |
...ud/appconfiguration/config/implementation/feature/entity/FeatureFilterEvaluationContext.java
Show resolved
Hide resolved
...ud/appconfiguration/config/implementation/feature/entity/FeatureFilterEvaluationContext.java
Outdated
Show resolved
Hide resolved
|
Do we need to update the Besides, is this |
| @@ -0,0 +1,78 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
Is there any difference between State and FeatureFlagState? Is it possible that we make State as base class and FeatureFlagState extend State?
https://github.com/Azure/azure-sdk-for-java/blob/feature/spring-boot-3/sdk/spring/spring-cloud-azure-appconfiguration-config/src/main/java/com/azure/spring/cloud/appconfiguration/config/implementation/State.java
There was a problem hiding this comment.
Pre this PR there was only one State. The idea was that State had the Configuration Setting that was the watch key, which contains the etag info and we would just go through the list to see if any of them changed. This also worked for feature flags.
Feature Flags now refresh by page, so we need to track the etags by the page, which is part of the MatchConditions, which is inside of a SettingSelector. If you look at the confusingly named FeatureFlags object which is new it contains this SettingSelector. This is how we now have to track changes for all feature flags.
I'm not a huge fan of this setup, there are a number of reasons why I have this up as a draft. This part will should also work when we add page monitoring for ConfigurationSettings, so we may want to update the names to reflect this.
| @@ -0,0 +1,168 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
Looks like we're moving the logic from AppConfigurationFeatureManagementPropertySource to this FeatureFlagClient, I'm not sure why we need to do this.
There was a problem hiding this comment.
FeatureFlagClient is here to solve the problem of de-duplicating feature flags loaded from multiple sources. They can come from multiple selects, multiple stores, even from Snapshots. We could have used the old property source class somewhat as is if it wasn't for Snapshots.
Though I just did notice an issue, we might have a logic issue when a store fails to load and we retry from a replica. Will look into.
| try { | ||
| client.listConfigurationSettings(settingSelector).streamByPage().forEach(pagedResponse -> { | ||
| checks.add( | ||
| new MatchConditions().setIfNoneMatch(pagedResponse.getHeaders().getValue(HttpHeaderName.ETAG))); |
There was a problem hiding this comment.
what is this checks used for? To check if there's a "ETAG" header?
There was a problem hiding this comment.
checks is a list of MatchConditions. The MatchConditions class contains the logic from the SDK for paged tags. if you look down a few lines we add the checks list to the SettingSelector. This modified SettingSelector will be the one used in refresh to check for changes.
...azure/spring/cloud/appconfiguration/config/implementation/AppConfigurationReplicaClient.java
Outdated
Show resolved
Hide resolved
Description
Updates Spring Provider to the V2 Feature Management schema.
https://github.com/Azure/AppConfiguration/blob/main/docs/FeatureManagement/FeatureManagement.v2.0.0.schema.json
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines