Skip to content

Add container view to container apps #673

Merged
motm32 merged 13 commits intomainfrom
meganmott/containers
Sep 19, 2024
Merged

Add container view to container apps #673
motm32 merged 13 commits intomainfrom
meganmott/containers

Conversation

@motm32
Copy link
Copy Markdown
Contributor

@motm32 motm32 commented Apr 5, 2024

Part of #426

Bringing this branch back from the dead 🧟

Here is how the view looks:
image

At the moment we are only supporting displaying the information. In the future we hope to add support for adding/uploading environment variables.

I also extended RevisionDraftDescendantBase instead of ContainerAppsItem so when we add further support it already basically has drafts/revisions support. Let me know if we want to change it to ContainerAppsItem instead.

@motm32 motm32 requested a review from a team as a code owner April 5, 2024 22:20
@MicroFish91
Copy link
Copy Markdown
Contributor

This is looking really nice 😎, can't wait to try it out later

@motm32
Copy link
Copy Markdown
Contributor Author

motm32 commented Apr 16, 2024

This is how it looks in the case of multiple containers:
image

Comment thread src/commands/registerCommands.ts
Comment thread src/tree/revisionManagement/RevisionItem.ts Outdated
Comment thread src/tree/containers/ContainerItem.ts Outdated
Comment thread src/tree/containers/ImagesItem.ts Outdated
Comment thread src/tree/containers/ImagesItem.ts Outdated
Comment thread src/tree/containers/EnvironmentVariablesItem.ts Outdated
Comment thread src/tree/containers/ImagesItem.ts Outdated
Comment thread src/tree/containers/EnvironmentVariableItem.ts Outdated
Comment thread src/tree/containers/ContainersItem.ts Outdated
@MicroFish91
Copy link
Copy Markdown
Contributor

MicroFish91 commented Jun 18, 2024

I did a quick test in multiple revisions mode and the container items and children still throw errors if expanded at the same time. I think you'll need to add a parent resource ID at the top level to ensure uniqueness. Here's one way to do it:

  1. ContainersItem
    a. From: ${containerApp.id}/containers
    b. To: ${this.parentResource.id}/containers

For resolving this.parentResource.id, refer to the implementation in ScaleRuleItem as an example.

  1. ContainerItem
    a. From: containerItem-${this.container.name}-${this.revision.name}
    b. To: parentId + /${container.id}

Where parentId is the ContainersItem id.

  1. EnvironmentVariablesItem
    a. From: ${this.containerId}/environmentVariables
    b. To: parentId + /environmentVariables

Where parentId is either the ContainersItem or ContainerItem id.

  1. EnvironmentVariableItem
    a. From: ${this.containerId}/environmentVariables/${this.envVariable.name}
    b. To: parentId + /${this.envVariable.name}

Where parentId is the EnvironmentVariablesItem id.

  1. ImageItem
    a. From: ${this.containerId}/image
    b. To: parentId + /image

Where parentId is either the ContainersItem or ContainerItem id.

  1. containerImageNameItem and containerImageRegistryItem
    Add id: ${this.id}/imageName and ${this.id}/imageRegistry, respectively.

Where this.id refers to the parent id, i.e. the parent ImageItem id.

Copy link
Copy Markdown
Contributor

@MicroFish91 MicroFish91 left a comment

Choose a reason for hiding this comment

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

LGTM, very excited to have this feature merged in, great job 🥳

@motm32 motm32 merged commit 0c05a41 into main Sep 19, 2024
@motm32 motm32 deleted the meganmott/containers branch September 19, 2024 22:15
@microsoft microsoft locked and limited conversation to collaborators Nov 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants