Add app config and related feature flag capabilities #87
Add app config and related feature flag capabilities #87aprilk-ms wants to merge 33 commits intoAzure-Samples:mainfrom aprilk-ms:aprilk/add-exp-clean
Conversation
aprilk-ms
commented
Apr 15, 2025
aprilk-ms
commented
- Updated bicep to include App Config provisioning
- Added github workflow to deploy feature flag config
- Integrated app config initialization/refresh process in the app code
- Fetch agent variant for the thread in chat call (fallback to default agent if app config or feature flag is not setup)
- On the UI side,
- added a new thread button to start a new thread (to see a new variant assigned), and
- show the variant name in the message title bar
infra/main.bicep
Outdated
|
|
||
| @description('Random seed to be used during generation of new resources suffixes.') | ||
| param seed string = newGuid() | ||
| param seed string = newGuid() // why do we need this? |
There was a problem hiding this comment.
Why we want this? Sometimes when there is an issue, I do azd up several times and it surprised me that new set of resources got created.
There was a problem hiding this comment.
I agree, this is strange. I created an issue in this repo about it
src/api/routes.py
Outdated
| def get_agent(request: Request) -> Agent: | ||
| return request.app.state.agent | ||
|
|
||
| def get_feature_manager(request: Request): |
There was a problem hiding this comment.
I did not add types for feature_manager and app_config, assuming that users might not have the modules installed. Is it the right practice?
There was a problem hiding this comment.
@aprilk-ms, wouldn't they have them installed from requirements.txt. Also, our provider implements dict, so it's safe to have it's type be Mapping[str, Union[str, JSON]].
There was a problem hiding this comment.
I agreed. I wanted to have the types but was not sure after seeing some lines in main.py catching ModuleNotFound issue. I added the types now and see what other reviewers think.
src/api/routes.py
Outdated
| # Attempt to get an existing thread. If not found, create a new one. | ||
| try: | ||
| if thread_id and agent_id == agent.id: | ||
| if thread_id: |
There was a problem hiding this comment.
Question: is there a problem to use different agents in the same thread? Why is this needed?
There was a problem hiding this comment.
If the agentId has been changed on the server side, I would like to avoid get_thread and avoid exception. But instead start a new chat by create_thread.
| opentelemetry-sdk | ||
|
|
||
| azure-appconfiguration-provider==2.1.0b1 | ||
| FeatureManagement==2.0.0b3 |
There was a problem hiding this comment.
TODO: update this after new release is out
src/api/routes.py
Outdated
| async def get_agent_variant(feature_manager, ai_client: AIProjectClient, thread_id: str): | ||
| if feature_manager: | ||
| # Fetch the variant for the feature flag "my-agent" using thread_id as targeting Id | ||
| agent_variant = feature_manager.get_variant("my-agent", thread_id) |
There was a problem hiding this comment.
@aprilk-ms I see you are doing async. You can get an async version from from featuremanagement.aio import FeatureManager and the same thing for our provider. from azure.appconfiguration.provider.aio import load. Should work the same as the rest of the azure sdk for python in async usage.
There was a problem hiding this comment.
Updated to async as suggested.
|
TODO: still need to update readme |
infra/main.bicep
Outdated
| } | ||
|
|
||
| module userRoleConfigStoreDataOwner 'core/security/role.bicep' = { | ||
| name: 'user-role-config-store-data-owner' |
There was a problem hiding this comment.
@howieleung I also noticed all the other userRole assignment above are using app service principal and not the caller. Is it intended?
There was a problem hiding this comment.
Yes, a while ago, I also had principalId as param at the main. But when I try to deploy from Portal UI, the service principal has role issues.
| credential=DefaultAzureCredential(), | ||
| feature_flag_enabled=True, | ||
| feature_flag_refresh_enabled=True, | ||
| refresh_interval=300 # no refresh within 5 mins to avoid throttling |
There was a problem hiding this comment.
TODO: update to 2 mins
|
|
||
| @description('Random seed to be used during generation of new resources suffixes.') | ||
| param seed string = newGuid() | ||
| param seed string = '' //newGuid() // why do we need this? |
There was a problem hiding this comment.
If you remove newGUID, it might break AI Search resource creation.
src/api/static/ChatUI.js
Outdated
| } | ||
|
|
||
| addNewThreadClickListener() { | ||
| const newThreadButton = document.getElementById('new-thread-button'); |
There was a problem hiding this comment.
What should happen if you refresh the browser and we call api to reload chat history?
4 participants
