Conditional Templates #2795
Conditional Templates #2795
Conversation
There was a problem hiding this comment.
I think this PR lends itself naturally to being split into several smaller PRs:
- Increasing the number of processes in resource processor (AFAIU it is not related to this pull request?)
- Endpoints for getting a template with a specific version
- Adding support for AllOf in the templates including the changes on UI necessary
- Replacing auto_create with Automatic and manual auth_type (am I right that it is just refactoring and does not depend on the rest of the PR?)
- Finally, changing the workspace templates to take advantage of the introduced AllOf feature for Airlock configuration
As it stands, I would be reluctant to approve this PR as there are too many changes. It is likely that I and other reviewers will miss something while reviewing.
It would also be good to call out these changes as separate items in CHANGELOG.
| @@ -130,13 +130,15 @@ async def create_review_user_resource( | |||
| # Getting the review configuration from the airlock request's workspace properties | |||
| if airlock_request.type == AirlockRequestType.Import: | |||
| config = workspace.properties["airlock_review_config"]["import"] | |||
There was a problem hiding this comment.
Do you need this line still?
There was a problem hiding this comment.
yep - as mentioned below, the structure is still:
properties: {
...
{
"airlock_review_config": {
"import": {
...
}, "export": { }
}
}
| workspace_id = config["workspace_id"] | ||
| workspace_id = config["import_vm_workspace_id"] | ||
| workspace_service_id = config["import_vm_workspace_service_id"] | ||
| user_resource_template_name = config["import_vm_user_resource_template_name"] | ||
| else: | ||
| assert airlock_request.type == AirlockRequestType.Export | ||
| config = workspace.properties["airlock_review_config"]["export"] |
There was a problem hiding this comment.
Same as above, do you need this line? AFAIU there won't be a field name "export" anymore?
There was a problem hiding this comment.
the nested structure is still there, so there are parent objects for import and export still.
api_app/api/routes/api.py
Outdated
| @@ -36,8 +36,11 @@ | |||
| core_router.include_router(health.router, tags=["health"]) | |||
| core_router.include_router(ping.router, tags=["health"]) | |||
| core_router.include_router(workspace_templates.workspace_templates_admin_router, tags=["workspace templates"]) | |||
| core_router.include_router(workspace_service_templates.workspace_service_templates_core_router, tags=["workspace service templates"]) | |||
|
|
|||
| # NOTE: User Resource Templates need to be registered before workspace service templates to cater for the `/{service_template_name}/user-resources` | |||
There was a problem hiding this comment.
This comment makes sense in the context of this PR I'm looking at, but I think it will make less sense when people just see it in the code. Maybe reword to something like this:
NOTE: User Resource Templates need to be registered before Workspace Service templates, as some user resource endpoints depend on workspace service endpoints.
|
/test-extended |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/3364875169 (with refid (in response to this comment from @damoodamoo) |
|
/test-destroy-env |
|
Destroying PR test environment (RG: rg-tre3b8e3eae)... (run: https://github.com/microsoft/AzureTRE/actions/runs/3369145827) |
|
Destroying branch test environment (RG: rg-treda8868a4)... (run: https://github.com/microsoft/AzureTRE/actions/runs/3369145827) |
|
Branch test environment destroy complete (RG: rg-treda8868a4) |
|
/test-extended |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/3369186464 (with refid (in response to this comment from @damoodamoo) |
|
PR test environment destroy complete (RG: rg-tre3b8e3eae) |
|
/test-destroy-env |
|
Destroying PR test environment (RG: rg-tre3b8e3eae)... (run: https://github.com/microsoft/AzureTRE/actions/runs/3369263912) |
|
PR test environment destroy complete (RG: rg-tre3b8e3eae) |
|
Destroying branch test environment (RG: rg-treda8868a4)... (run: https://github.com/microsoft/AzureTRE/actions/runs/3369263912) |
|
Branch test environment destroy complete (RG: rg-treda8868a4) |
|
/test-extended |
|
🤖 pr-bot 🤖 🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/3369300342 (with refid (in response to this comment from @damoodamoo) |
|
Closing as env is messed up. Creating new. |
Resolves #2741
allOffeature of JSON schema to allow conditional fields + conditional schema validation