Skip to content

Changes to improve deployability - #349

Open
Pamela Fox (pamelafox) wants to merge 2 commits into
Azure-Samples:mainfrom
pamelafox:deploymentchanges
Open

Changes to improve deployability#349
Pamela Fox (pamelafox) wants to merge 2 commits into
Azure-Samples:mainfrom
pamelafox:deploymentchanges

Conversation

@pamelafox

Copy link
Copy Markdown

Purpose

This PR introduces various changes to improve the deployability of the repository and cleanup deleted/commented code.

Specifically:

  • Removed main.bicepparam, as that's incompatible with the location Myouki (@Allowed) decorator (it wont enforce it)
  • Removed ai.yaml, as that's incompatible with using "azd env set" to override values (as far as I know)
  • Added more azd env variables for the AI deployments to main.parameters.json

I also changed how credentials are handled:

  • The old code called DefaultAzureCredential multiple times. We should only call a credential once, and reuse that credential, as it takes time to acquire a credential.
  • We should not call DefaultAzureCredential as it's too ambiguous, and we know what credentials to use instead. I replaced it where I could with a function that uses a ChainedTokenCredential of ManagedIdentityCredential and AzureDeveloperCliCredential.
  • I changed the Bicep to output the AZURE_TENANT_ID and pass that into AzureDeveloperCliCredential so that you can deploy this even if you're not using your default tenant.
  • Unfortunately, both Prompty and the azure-ai-evaluation SDK do not yet allow you to pass in a credential, so I couldn't fully move away from DefaultAzureCredential. I am in talks with both SDKs about allowing that, since this template is nearly unusable with the DAC usage.

Does this introduce a breaking change?

[ ] Yes ? I'm not sure if it affects workshop use?
[ ] No

Pull Request Type

What kind of change does this Pull Request introduce?

[X] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[X] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • azd up
  • Run the local app. Unfortunately, since I dont have 100K TPM, I couldnt get pass the researcher phase.

@marlenezw

Copy link
Copy Markdown
Contributor

Thanks Pamela, going to test and then will merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants