Skip to content

add profile support for docker-compose down#4467

Merged
bwateratmsft merged 5 commits intomicrosoft:mainfrom
PotatoZhou:gjzhou/compose-down-profile
Mar 10, 2025
Merged

add profile support for docker-compose down#4467
bwateratmsft merged 5 commits intomicrosoft:mainfrom
PotatoZhou:gjzhou/compose-down-profile

Conversation

@PotatoZhou
Copy link
Contributor

for issue #4458

@PotatoZhou PotatoZhou requested a review from a team as a code owner January 14, 2025 01:01
@PotatoZhou
Copy link
Contributor Author

@microsoft-github-policy-service agree

@bwateratmsft
Copy link
Collaborator

Thanks for the contribution @PotatoZhou! We're right in the middle of releasing 1.29.4 so we're going to hold off on this PR until that release is done, but at a glance everything looks good.

@bwateratmsft
Copy link
Collaborator

So this is different from what #4458 is asking for--that issue is asking for the docker-compose down VSCode task to support profiles. Your PR is still valid but is adding a customizable command instead of enhancing the docker-compose down task.

Also, we've made some changes in this area recently that would affect your PR--can you merge main and update compose.ts' compose() function? It now references 'upSubset' directly.

@PotatoZhou
Copy link
Contributor Author

sure, will do

Copy link
Collaborator

@bwateratmsft bwateratmsft left a comment

Choose a reason for hiding this comment

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

I don't want to change the existing Compose Down command. If your goal is to fix #4458, you want to do the VSCode task, not the customizable command. The code for that is partly in package.json's contributes.taskDefinitions -> docker-compose and partly in src/tasks.

@PotatoZhou
Copy link
Contributor Author

correct me if I am wrong :) I believe that the client.down (in DockerComposeTaskProvider.ts) does not support profiles, I checked ContainerOrchestratorClient.ts of vscode-container-client and in type DownCommandOptions, "profiles" is not presented, Should I report this as an issue over there?

saltman007web

This comment was marked as spam.

@bwateratmsft
Copy link
Collaborator

@PotatoZhou you are right, good catch. We will need to add support for profiles in the compose down function in ContainerOrchestratorClient. Can you file an issue on https://github.com/microsoft/vscode-docker-extensibility/issues?

Copy link
Collaborator

@bwateratmsft bwateratmsft left a comment

Choose a reason for hiding this comment

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

These changes look good. I'll try to cut a release of the @microsoft/vscode-container-client package soon.

@bwateratmsft bwateratmsft self-requested a review February 21, 2025 18:21
@bwateratmsft
Copy link
Collaborator

@PotatoZhou @microsoft/vscode-container-client version 0.2.0 is available with the changes to support profiles in compose down commands. You can update your PR to take advantage of that.

@bwateratmsft bwateratmsft linked an issue Mar 10, 2025 that may be closed by this pull request
Copy link
Collaborator

@bwateratmsft bwateratmsft left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for the contribution @PotatoZhou!

@bwateratmsft bwateratmsft merged commit c7f7128 into microsoft:main Mar 10, 2025
2 checks passed
@bwateratmsft bwateratmsft added this to the 1.30.0 milestone Mar 10, 2025
@bwateratmsft bwateratmsft modified the milestones: 1.30.0, 1.29.5 Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dockerCompose down task does not have a profiles argument

4 participants