-
Notifications
You must be signed in to change notification settings - Fork 155
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
Fix Cosmos DB local debugging #4340
Fix Cosmos DB local debugging #4340
Conversation
…robi/add-cosmos-data-contributor
Unit Test Results608 tests 608 ✅ 6s ⏱️ Results for commit 5bb2154. ♻️ This comment has been updated with latest results. |
…robi/add-cosmos-data-contributor
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.
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
Files not reviewed (1)
- devops/scripts/setup_local_debugging.sh: Language not supported
Comments suppressed due to low confidence (3)
api_app/api/dependencies/database.py:29
- The removal of the dynamic key fetching logic using CosmosDBManagementClient means that the key must always be available in the configuration. This could cause issues if the key is not provided.
if STATE_STORE_KEY:
api_app/api/dependencies/database.py:47
- The removal of the MANAGED_IDENTITY_CLIENT_ID check might affect the behavior if the managed identity is required but not explicitly checked.
credential = await get_credential_async()
api_app/api/dependencies/database.py:29
- Ensure that the new connection logic, especially the use of STATE_STORE_KEY and managed identity, is covered by tests.
if STATE_STORE_KEY:
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.
LGTM
/test |
🤖 pr-bot 🤖 🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/13367437668 (with refid (in response to this comment from @marrobi) |
/test-force-approve https://github.com/microsoft/AzureTRE/actions/runs/13367437668 |
🤖 pr-bot 🤖 ✅ Marking tests as complete (for commit 5bb2154) (in response to this comment from @marrobi) |
Introduce functionality to assign the Cosmos DB Built-in Data Contributor role when configuring local debugging to so that cosmos data can be read.
Always use Entra ID credentials unless a key is manually specified.