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

Open AI Workspace Service #4075

Merged

Conversation

harryy94
Copy link
Contributor

Addition of Open AI Workspace Service for #3810

This builds upon @marrobi 's original draft PR.

We are able to install an Open AI service and access it via the VM, and are still in progress of doing more tests, but welcome any feedback in the meantime!

@github-actions github-actions bot added the external PR from an external contributor label Aug 21, 2024
Copy link

github-actions bot commented Aug 21, 2024

Unit Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 3669487.

♻️ This comment has been updated with latest results.

@marrobi marrobi mentioned this pull request Aug 21, 2024
5 tasks
@harryy94 harryy94 marked this pull request as ready for review August 23, 2024 11:23
@marrobi
Copy link
Member

marrobi commented Aug 29, 2024

Works great for me, once I sorted quota issues out. I had to manually set the version, as the default "auto update" selection didn;t work, seemed to be trying to use a version not available in the region.

image

Blocked external:

image

Works internal:
image

We should surface the API key somehow as a later issue as per #2401

Thanks!

Copy link
Member

@marrobi marrobi left a comment

Choose a reason for hiding this comment

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

LGTM

@marrobi
Copy link
Member

marrobi commented Aug 29, 2024

@harryy94 can you add the tags, not sure what was wrong before, but should support them:
image

@harryy94 harryy94 force-pushed the feat/openai-workspace-service branch from c34c069 to 3669487 Compare September 2, 2024 09:23
@harryy94
Copy link
Contributor Author

harryy94 commented Sep 2, 2024

@harryy94 can you add the tags, not sure what was wrong before, but should support them: image

Hi, thanks for your review :) For the above I realised I forgot this line

tags                          = local.workspace_service_tags

so I've added that in on the OpenAI instance. The other error I was getting was I accidentally tried to add tags on the deployment instead of the instance which doesn't support tags.

@marrobi
Copy link
Member

marrobi commented Sep 2, 2024

/test-extended 3669487

Given base workspace and core changes doing full test.

Copy link

github-actions bot commented Sep 2, 2024

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/10665019851 (with refid cc34db55)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Sep 2, 2024

/destroy-test-env

Copy link

github-actions bot commented Sep 2, 2024

🤖 pr-bot 🤖

/destroy-test-env is not recognised as a valid command.

You can use the following commands:
    /test - build, deploy and run smoke tests on a PR
    /test-extended - build, deploy and run smoke & extended tests on a PR
    /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
    /test-shared-services - test the deployment of shared services on a PR build
    /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
    /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
    /help - show this help

(in response to this comment from @marrobi)

@marrobi
Copy link
Member

marrobi commented Sep 2, 2024

/test-destroy-env

Copy link

github-actions bot commented Sep 2, 2024

Destroying PR test environment (RG: rg-trecc34db55)... (run: https://github.com/microsoft/AzureTRE/actions/runs/10668007159)

Copy link

github-actions bot commented Sep 2, 2024

PR test environment destroy complete (RG: rg-trecc34db55)

@tim-allen-ck tim-allen-ck self-requested a review September 3, 2024 08:49
@tim-allen-ck
Copy link
Collaborator

I seem to keep hitting this error:

 Error: Failed to download module
│ 
│ Could not download module "terraform_azurerm_environment_configuration" (main.tf:30) source code from
│ "git::https://github.com/microsoft/terraform-azurerm-environment-configuration.git?ref=0.6.0": error downloading
│ 'https://github.com/microsoft/terraform-azurerm-environment-configuration.git?ref=0.6.0': /usr/local/bin/git exited with 128: fatal: detected
│ dubious ownership in repository at
│ '/workspaces/AzureTRE/templates/workspace_services/openai/terraform/.terraform/modules/terraform_azurerm_environment_configuration'
│ To add an exception for this directory, call:
│ 
│       git config --global --add safe.directory
│ /workspaces/AzureTRE/templates/workspace_services/openai/terraform/.terraform/modules/terraform_azurerm_environment_configuration
│ 

Any ideas to add it permanently in?

@marrobi
Copy link
Member

marrobi commented Sep 3, 2024

detected
│ dubious ownership in repository at

This in the dev container when building? I didn't get that. Weird.

That module is used by other bundles too, so weird just seeing it now.

@tim-allen-ck
Copy link
Collaborator

detected
│ dubious ownership in repository at

This in the dev container when building? I didn't get that. Weird.

That module is used by other bundles too, so weird just seeing it now.

Yeah I figured it was used elsewhere.
Yes, doing a container deploy I ended up doing it multiple times for different bundles in the end.

Copy link
Collaborator

@tim-allen-ck tim-allen-ck left a comment

Choose a reason for hiding this comment

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

LGTM

@tim-allen-ck tim-allen-ck merged commit 4040e72 into microsoft:main Sep 16, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external PR from an external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants