Skip to content
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

Fully azdevify the application #281

Merged
merged 72 commits into from
Feb 21, 2024
Merged

Fully azdevify the application #281

merged 72 commits into from
Feb 21, 2024

Conversation

jongio
Copy link
Member

@jongio jongio commented Feb 6, 2024

This is a large PR, but was necessary because of all the additions/improvements that will come and need to be tested as a unit.

  1. Adds infra/core bicep files
  2. Adds Azure Developer CLI support
  3. Optionally use KV

To effectively share code, we moved some projects into subfolders, so please see that.

Here are the tests:

With RBAC

AZURE_AUTH_TYPE= rbac
USE_KEY_VAULT= false
With Keys and not use Key Vault

AZURE_AUTH_TYPE= keys
USE_KEY_VAULT= false
With Keys and use Key Vault

AZURE_AUTH_TYPE= keys
USE_KEY_VAULT= true
Note: The current default values ​​are: AZURE_AUTH_TYPE=keys and USE_KEY_VAULT=true.

You can test with azd init -t jongio/chat-with-your-data-solution-accelerator

@adamdougal
Copy link
Collaborator

This problem should be solved if we place the tests in the code folder. What do you think?

I think that's okay.

As an alternative, though it might break the deployment. If you change everywhere in app.py that imports from the backend dir to be prepended with code it fixes the issue. E.g.

Change this:

from backend.batch.utilities.helpers.EnvHelper import EnvHelper

To this:

from code.backend.batch.utilities.helpers.EnvHelper import EnvHelper

I believe this is because of line 17 of the app.py:

sys.path.append(os.path.join(os.path.dirname(__file__), ".."))

This fixes the tests, however I've not tested the deployment with this change.

@zedy-wj
Copy link
Contributor

zedy-wj commented Feb 19, 2024

Sorry for the delay.

To this:

from code.backend.batch.utilities.helpers.EnvHelper import EnvHelper

I tested the approach like this and for tests it worked for me, but unfortunately azd is still not able to deploy.

@ross-p-smith ,@adamdougal - At present, I moved the tests folder to the codes folder. Following the steps as below, I can run the test successfully in local, but it fails in CI. Is it a problem with the root path?
image

@ross-p-smith ross-p-smith removed a link to an issue Feb 19, 2024
8 tasks
@ross-p-smith ross-p-smith mentioned this pull request Feb 19, 2024
8 tasks
@ross-p-smith
Copy link
Collaborator

At present, I moved the tests folder to the codes folder.

I'm not sure we want to rename the code folder to codes. If we need to move the tests folder then this is OK, as long as it is not alongside the code that it is testing so that it does not get copied into the docker images.

@zedy-wj
Copy link
Contributor

zedy-wj commented Feb 20, 2024

I'm not sure we want to rename the code folder to codes.

@ross-p-smith - Sorry, the folder name has been changed back. Currently, we move tests folder under code folder, through the following modifications, CI can pass normally.
image

@ross-p-smith ross-p-smith self-requested a review February 20, 2024 20:29
Copy link
Collaborator

@ross-p-smith ross-p-smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you ever so much for this - I understand that this took great effort

@adamdougal adamdougal dismissed their stale review February 21, 2024 08:57

Comments adressed

@ross-p-smith ross-p-smith added this pull request to the merge queue Feb 21, 2024
Merged via the queue into Azure-Samples:main with commit 2623e1b Feb 21, 2024
3 checks passed
eduardogch pushed a commit to devopsdale/chat-with-your-data-solution-accelerator that referenced this pull request Apr 30, 2024
* azdevify this sample and add azd new feature in devcontainer

* instead parameters.json file with .bicepparam

* processing format

* keep consistent casing for params

* move appsettings

* remove some invalid changes

* fix format issues

* Remove depandsOn

* remove resources file

* remov keys from output

* remove existing reference in module

* update ReadMe

* add rgName in listkeys

* add use keyvault option

add keyvault option to get secret keys

remove invalid changes

add param useKeyVault

* set default value in .bicepparam

* fix some bugs

* update rbac

* update rbac

* fix some format and update infra core

* Update openai to v1

* use resourcegroup.name() instead pass in rhName

* Revert "Update openai to v1"

* add token provider

* Update requirements.txt

* revert

* fix chat

* Update TextProcessingTool.py

* update KeyVault options

* abstract auth type with define method

* remove invalid infra/core

* update readme and storage.bicep in infra/core

* Update output format

* default to use rbac

* add some comments

* add readme and speech service related code in main.bicep

* fix azd deploy problem

* move code folder structure

* fix ci error and some format issue

* fix some format issue

* fix some format issues

* little changes

* fix backend issue

* Fixed mismatched requirements

* Fix build-frontend

* fix function error

* Run black against the code base

* try to fix ci error

* fix ci errors

* fix azd deploy web error

* fix web debug error and update main.bicep

* resolve conflict

* fix tests path

* try to fix CI/build error

* Merge main

* Fixed Requirements

* Black fixes

---------

Co-authored-by: zedy <zedy@wicresoft.com>
Co-authored-by: ChenxiJiang333 <v-chenjiang@microsoft.com>
Co-authored-by: Wenjie Yu(MSFT) <81678720+zedy-wj@users.noreply.github.com>
Co-authored-by: ChenxiJiang333 <119990644+ChenxiJiang333@users.noreply.github.com>
Co-authored-by: ross-p-smith <rosmith@microsoft.com>
Co-authored-by: Ross Smith <ross-p-smith@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
6 participants