-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Azdevify chat-with-your-data-solution-accelerator. #2
Merged
Merged
Changes from 36 commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
24be759
azdevify this sample and add azd new feature in devcontainer
fbfd9b7
instead parameters.json file with .bicepparam
d32d977
processing format
552f254
keep consistent casing for params
c00f55a
move appsettings
b2e62b2
remove some invalid changes
b1a97b7
fix format issues
625487e
Remove depandsOn
65ac3c5
remove resources file
43b09e7
remov keys from output
bd29d31
remove existing reference in module
8587450
update ReadMe
afd3ca8
add rgName in listkeys
654762e
add use keyvault option
cc3e957
set default value in .bicepparam
9dc6a78
fix some bugs
9bcb2a7
update rbac
1a9212b
update rbac
eb11eab
fix some format and update infra core
a403d25
Update openai to v1
ChenxiJiang333 b88ee5f
use resourcegroup.name() instead pass in rhName
4bdf8c3
Merge pull request #1 from zedy-wj/openaiv1
zedy-wj 1ecb9be
Revert "Update openai to v1"
zedy-wj 75f230e
Merge pull request #2 from zedy-wj/revert-1-openaiv1
zedy-wj 969aa25
add token provider
ChenxiJiang333 ffee3f8
Merge branch 'azdevify' into openaiv1
ChenxiJiang333 778e1d4
Merge pull request #3 from zedy-wj/openaiv1
zedy-wj 5892d5d
Update requirements.txt
ChenxiJiang333 ce7d48d
Merge pull request #4 from zedy-wj/ChenxiJiang333-patch-1
zedy-wj 2b683de
revert
397ab11
fix chat
ChenxiJiang333 1e21f2a
Update TextProcessingTool.py
ChenxiJiang333 9481a66
Merge pull request #5 from zedy-wj/newcommit
zedy-wj b25cce5
update KeyVault options
332402a
abstract auth type with define method
bd092df
remove invalid infra/core
5406c5a
update readme and storage.bicep in infra/core
d2145ec
Update output format
c83a409
default to use rbac
aa86081
add some comments
06bb879
solve conflicts
ChenxiJiang333 1a7303a
add readme and speech service related code in main.bicep
2c8abfb
fix azd deploy problem
5dcf10c
move code folder structure
779065f
fix conflict
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
{ | ||
"name": "Chat with your data Solution Accelerator", | ||
"image": "mcr.microsoft.com/devcontainers/python:3.11", | ||
"features": { | ||
"ghcr.io/devcontainers/features/node:1": { | ||
"version": "16", | ||
"nodeGypDependencies": false | ||
}, | ||
"ghcr.io/devcontainers/features/powershell:1.1.1": {}, | ||
"ghcr.io/devcontainers/features/azure-cli:1.2.1": {}, | ||
"ghcr.io/azure/azure-dev/azd:latest": {} | ||
}, | ||
"customizations": { | ||
"vscode": { | ||
"extensions": [ | ||
"ms-azuretools.azure-dev", | ||
"ms-azuretools.vscode-bicep", | ||
"ms-python.python", | ||
"esbenp.prettier-vscode" | ||
] | ||
} | ||
}, | ||
"forwardPorts": [ | ||
50505 | ||
], | ||
"postCreateCommand": "", | ||
"remoteUser": "vscode", | ||
"hostRequirements": { | ||
"memory": "8gb" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# yaml-language-server: $schema=https://raw.githubusercontent.com/Azure/azure-dev/main/schemas/v1.0/azure.yaml.json | ||
|
||
name: chat-with-your-data-solution-accelerator | ||
metadata: | ||
template: [email protected] | ||
services: | ||
web: | ||
project: . | ||
language: py | ||
host: appservice | ||
hooks: | ||
prepackage: | ||
windows: | ||
shell: pwsh | ||
run: cd ./frontend;npm install;npm run build | ||
interactive: true | ||
continueOnError: false | ||
|
||
adminweb: | ||
project: ./backend | ||
language: py | ||
host: appservice | ||
|
||
function: | ||
project: ./backend | ||
language: py | ||
host: function |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
import os | ||
import openai | ||
import logging | ||
import re | ||
import json | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you investigate this flow:
The above adds more complexity, but allows the user to store the keys in KV as an option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just linking this as we should revisit the Speech Service using this method too: - Azure-Samples#101 (not for this PR!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use of
KeyVault
has been added and is currently testing well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the back and forth on this. Let's do this:
Default to using rbac.
If AUTH_TYPE is rbac, then assume the developer wants to also use key vault. Put the keys in keyvault and then read them from keyvault.
If the user sets AUTH_TYPE to keys, then read the keys from the environment and set the keys using listKeys function.
I think that will be cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our understanding, if
AUTH_TYPE
isrbac
, then someApp Services
andUser
should be assigned corresponding permissions in themain.bicep
file. This means that keys will no longer be needed in the code, but the service will be accessed directly through something likeDefaultAzureCredential()
.So regarding what you mention here,
If AUTH_TYPE is rbac, then assume the developer wants to also use key vault.
, which confuses us. Because in our concept, one of them is enough.If we have any misunderstandings about
rbac
or your comments, please let me know, thanks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, only use a key vault if there's no RBAC option for the service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the logic in our code so far, please re-review it, thanks!
Default to using rbac: https://github.com/jongio/chat-with-your-data-solution-accelerator/pull/2/files#diff-c274a6091f4ca06948f0fe1ab8681ec4eb6b4e98c4be09717a2e3cacd1344727R9-R12
If AUTH_TYPE is rbac, the developer must choose not to use the KeyVault, if AUTH_TYPE is keys, the developer can choose to use the KeyVault or directly use
listKeys()
.